Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🪲 Not possible to correctly use as Editor for Git commands #678

Open
ADTC opened this issue Dec 6, 2024 · 14 comments
Open

🪲 Not possible to correctly use as Editor for Git commands #678

ADTC opened this issue Dec 6, 2024 · 14 comments

Comments

@ADTC
Copy link

ADTC commented Dec 6, 2024

Operating System and Version

macOS Sonoma 14.7.1 (23H222)

Distribution

brew

Description

I have tried configuring NotepadNext as the Editor for Git commands:

[core]
	editor = open -a Notepadnext

This doesn't work because:

  • The open command returns immediately, thus Git cannot wait for user input.
  • It looks like there's a bug in NotepadNext which does some weird things with the contents of the file opened and saves those changes, causing Git to act weird when it continues processing the command.
[core]
	editor = open -W -a Notepadnext

This kind of works:

  • The file for user input correctly opens in NotepadNext and the Git command is waiting for the user to finish editing it.
  • The file doesn't show any effects of the aforementioned bug. There are no weird automatic changes.

But the problem is:

  • If I close the tab with the file used by Git for user input, Git is still waiting for me to finish editing it.
  • I have to close and quit NotepadNext entirely to signal to Git that I'm done, so Git will continue its processing.

Is there another way? (The way TextMate does this kind of thing is, it has its own CLI binary which I can configure as the editor: /usr/local/bin/mate -w and it signals to Git that I'm done when I close the tab.)

PS: An alternative way to configure editor is to edit your .zshrc or .bashrc file to add this:

export EDITOR="open -W -a Notepadnext"

But the problems described above will persist.

Steps to Reproduce

  1. Be a Git user.
  2. Attempt to configure NotepadNext as editor for Git commands.
  3. Try to use a Git command that opens an editor for input. (Commands like commit and rebase typically do this.)

Additional Details

No response

@dail8859
Copy link
Owner

dail8859 commented Dec 6, 2024

This doesn't work because:
The open command returns immediately, thus Git cannot wait for user input.

My understanding is this how git works. It launches a process and waits for it to exit.

It looks like there's a bug in NotepadNext which does some weird things with the contents of the file opened and saves those changes, causing Git to act weird when it continues processing the command.

Can you define what "weird things" are happening?

This kind of works:
The file for user input correctly opens in NotepadNext and the Git command is waiting for the user to finish editing it.

Again this is my expectation that this is how git works.

But the problem is:
If I close the tab with the file used by Git for user input, Git is still waiting for me to finish editing it.
I have to close and quit NotepadNext entirely to signal to Git that I'm done, so Git will continue its processing.

Correct because the Notepad Next process is still running.

/usr/local/bin/mate -w and it signals to Git that I'm done when I close the tab

Interesting idea. Sounds like there would have to be some interprocess communication going on for git to wait on process a have it run process Notepad Next with the file. Then Notepad Next would close the file and signal process a, which would then die off, thus signaling git it is done.

That being said it sounds quite intricate and hard to implement cross platform this way.

With Notepad++ you can pass in the correct command line parameters to launch it in a stand alone isolated way. See this doc

This way wouldn't be super easy with Notepad Next but would probably be a better way to implement it.

@dail8859
Copy link
Owner

dail8859 commented Dec 6, 2024

All that being said if there was some way to signal git rather than the extra interprocess communication I'd be willing to consider that but currently that's outside my realm of expertise :)

@ADTC
Copy link
Author

ADTC commented Dec 6, 2024

My understanding is this how git works. It launches a process and waits for it to exit.
Again this is my expectation that this is how git works.

Correct.

Can you define what "weird things" are happening?

Never mind it, probably unimportant and not directly related to NotepadNext. Git deletes the rebase to-do file as soon as it processes it so this could be an anomaly of conflict between Git deleting it and NotepadNext reading it. I tried it with VS Code and the file just turned up blank.

Sounds like there would have to be some interprocess communication going on for git to wait on process a have it run process Notepad Next with the file. Then Notepad Next would close the file and signal process a, which would then die off, thus signaling git it is done.

Yes, this is how most editors tackle this scenario. The CLI binary slave also doubles as a convenient way to launch the editor from Terminal without have to create a custom alias for the open -a command.

That being said it sounds quite intricate and hard to implement cross platform this way.

In my experience, it looks like a CLI slave binary is only needed on macOS to launch an application (.app) and interface with it from the Terminal. In other OS like Windows, you can launch the application (.exe) directly and it can even run in a master-slave configuration (if coded properly). Chrome typically has several slave .exe processes running in Windows.

With Notepad++ you can pass in the correct command line parameters to launch it in a stand alone isolated way.

That is the odd one of the bunch, as the others simply have a CLI binary slave with a wait option that waits for the tab in the main app to be closed.

It doesn't look like NotepadNext has any capability of opening a second window in another process. Even if I use open -W -F it still attaches to the existing process, so I have to close the whole window to signal back that I'm done.

It looks like CotEditor is the closest to the ideal solution.

  • NotepadNext has the above discussed problems but will open in foreground.
    • Side issue: NotepadNext doesn't have syntax highlighting for Git-related files (like interactive rebase or commit message).
  • CudaText has the same issues as it similarly doesn't have any CLI slave tool and only suggests using an alias for open.
  • VS Code (code -w) seems to open in the background only, which is very annoying.
  • TextMate (mate -w) will only open in the foreground if it's already running. If it's not, then it opens in background. Also, TextMate is slow to launch - so slow that sometimes Git gives up waiting for it.
  • CotEditor (cot -w) seems to consistently open in foreground regardless of whether it's running or not. It's faster than TextMate.

I think for now I'll use CotEditor for giving text-file based input for Git commands, though I won't be using it as my default text editor.

if there was some way to signal git

The only way to signal Git is to kill the process that Git launched as its editor.

@ADTC
Copy link
Author

ADTC commented Dec 7, 2024

I have to add that it's a shame that I can't use NotepadNext for this purpose because it launches really fast from "not running" when invoked in Terminal using open -a. Even CotEditor is slower to do so (using cot).

If you can fix this somehow (perhaps make a little CLI binary slave for waiting on a tab), add support for Dark Mode, and optionally introduce a way to add syntax highlighting for Git's user input files, it will be perfect.

@dail8859
Copy link
Owner

dail8859 commented Dec 7, 2024

That is the odd one of the bunch

True, but the implementation is likely much easier than other alternatives.

I have to add that it's a shame that I can't use NotepadNext for this purpose

I'm willing to consider a pull request if you have any tangible solutions. I have a couple shots in the dark I can try but doubt they would result in a valid solution. The other difficulty is that this application intentionally tries to run in a "single instance" mode. A cross-platform solution will likely be difficult to create from scratch. I also only use Window's so I wouldn't be able to test on other operating systems.

@ADTC
Copy link
Author

ADTC commented Dec 7, 2024

True, but the implementation is likely much easier than other alternatives.

Not sure if it's possible to run a second instance of a .app application in macOS while one is already running. So while it may work in Windows, it may not work in macOS.

Is a CLI binary really more difficult? Maybe Qt has some way for a CLI binary to link with the running application and signal back and forth?

I'm willing to consider a pull request if you have any tangible solutions.

Absolutely clueless when it comes to C++ and Qt, etc. Sorry.

I wouldn't be able to test on other operating systems.

I can help with some testing on macOS.

@dail8859
Copy link
Owner

dail8859 commented Dec 8, 2024

Is a CLI binary really more difficult?

Building a cli binary is not, but doing the communication between the processes is the challenge. I'm not saying it's impossible, I've just not had any experience doing this or know the best way to go about implementing it.

@ADTC
Copy link
Author

ADTC commented Dec 8, 2024

I don't have deep knowledge of C++ but from what I know and what I just looked up on Stack Overflow, you should be able to do this:

  1. CLI binary is launched by Git or manually by user. (Let's call it nn for example. Maybe nn -w or nn --wait will be the way to start this process.)
  2. CLI binary tells NotepadNext the path of file to open AND its own PID (process ID). If NotepadNext is not already running, it launches.
  3. When the user closes the tab with the file opened in last step, NotepadNext will send a SIGTERM to the PID. (or is it SIGINT?)
  4. CLI binary receives the SIGTERM and promptly terminates. This will return the control to user or let Git continue.

All OS will have process ID and Linux is also UNIX based so SIGTERM or SIGINT will work. Windows might need some specialized way to kill process, I'm not sure. I know that any binary in Linux and any Windows exe file can be invoked with arguments, but for macOS does open -a pass arguments to the application? Idk I have to check. Maybe, because the file path is an argument, so perhaps the process ID can be an argument too.

Anyway that's it. That's the whole idea based on this: https://stackoverflow.com/a/69789890 (check out the link in the answer).

@dail8859
Copy link
Owner

dail8859 commented Dec 8, 2024

you should be able to do this:

This is one potential way. But I'm not confused on what the process is. What I dont know is how to implement it.

For example a few questions I have (I know you don't have the answers either, but you have no technical details in your proposal):

CLI binary tells NotepadNext the path of file to open

But how does it do this? QSharedMemory (which has issues with newer versions of Qt)? QLocalSocket? Some other way?

NotepadNext will send a SIGTERM to the PID. (or is it SIGINT?)

Does Qt have a cross platform way to do this? If the implementation is OS specific that leads to more possible issues to make sure it is tested cross platform.

This will return the control to user or let Git continue.

Git also needs to know if the file was saved or not. So it needs a specific return value.


Again, I understand the process just not the technical details or best way to implement it. The other thing that makes this difficult is the SingleApplication library that ensures only 1 Notepad Next process can exist, which means a secondary instance will kill itself off currently.

@ADTC
Copy link
Author

ADTC commented Dec 9, 2024

CLI binary tells NotepadNext the path of file to open

But how does it do this?

Don't know any of those Qt specific stuff, but I imagine that the CLI binary is a very simple binary which invokes this shell command:

open -a Notepadnext --args --invoker-pid 12345 /path/to/file

If I do open -a Notepadnext /path/to/file right now, it already works this way:

  1. If Notepadnext is not running, launch it. (It will restore session if that's enabled.)
  2. In the running Notepadnext, add a new tab for the file (or select existing tab for the file).

In Windows it should be similar to doing NotepadNext.exe /path/to/file. Try it in Command Prompt.

So I just imagine that Notepadnext just has to do one more thing:

  • Remember that the PID 12345 belongs to the file/tab invoked, and when the tab is closed, send a SIGINT or SIGTERM message to the PID. Then that process will exit.

only 1 Notepad Next process can exist, which means a secondary instance will kill itself off currently

The open -a command already works this way, as I described above. So this is already taken care of.

NotepadNext will send a SIGTERM to the PID. (or is it SIGINT?)

Does Qt have a cross platform way to do this?

Not sure about Qt, but I imagine the commands described in this page are cross-platform. Aren't they?

Git also needs to know if the file was saved or not. So it needs a specific return value.

No need. This is a misunderstanding. Neither the CLI binary process nor the NotepadNext process need to directly communicate with Git in any way. It's actually indirect. Git only waits for the invoked editor process to terminate. Once the process terminates, Git takes the file and continues doing its thing. It doesn't care whether the user chose to save the file or not save it. If the user discarded their changes, Git simply reads the file as it was last saved. (In fact, the user can close the file without making any changes at all, and that's also a perfectly valid response.)

This implementation is actually use-case-agnostic. You provide the feature without knowing how it is used and who is using it. In fact, Git is only one example use-case. Another is that I could simply invoke nn -w in Terminal and wait. Yet another is, I could have a shell script which invokes $EDITOR -w and it waits for me to edit a file and exit the editor before continuing. It's also possible that there are other tools besides Git that invokes the $EDITOR (or %EDITOR% in Windows). So we have to do this with the assumption that we don't know who is using it.

@ADTC
Copy link
Author

ADTC commented Dec 9, 2024

In addition to the above comment:

So it needs a specific return value.

I should clarify that while the CLI binary stub doesn't need to tell Git about whether the file was saved or not, it should still exit with a "non-zero" error code (usually just -1) if some exceptional situation happens. Like the following:

  • Notepadnext was uninstalled or doesn't exist in the system.
  • Notepadnext has crashed or somehow failed to launch.
  • Notepadnext couldn't launch because the .app file was renamed or moved.
  • The cat 🐈 pulled the plug 🔌 and now the computer 🖥️ is dead 💀 ( joke 😄 )

When the CLI binary process exits with a non-zero error code, Git will also halt its processing and give an error to the user that the editor could not be launched.

@ADTC
Copy link
Author

ADTC commented Dec 9, 2024

Additional comment: man open shows:

--args - All remaining arguments are passed to the opened application in the argv parameter to main(). These arguments are not opened or interpreted by the open tool.

So it should be:

open -a Notepadnext --args --invoker-pid 12345 /path/to/file

Whatever comes after --args gets passed to the Notepadnext application process.

Windows doesn't need this --args thing btw. It's only macOS (and Linux? not sure. Check in WSL2).

@ADTC
Copy link
Author

ADTC commented Dec 14, 2024

Do any of those comments make any sense? I'm no expert in any of these, but I'm only speaking from my experience as a software engineer.

@dail8859
Copy link
Owner

Don't know any of those Qt specific stuff

The problem is I don't either. Overall it makes sense but I don't know how to best do this with Qt. And the single instance library makes this more difficult.

So I won't be able to implement this myself. Hopefully someone else will be able to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants