subnut / nvim-ghost.nvim

:ghost: GhostText plugin for Neovim with zero dependencies :tada: Supports neovim running inside WSL too! :partying_face: Windows/Linux/macOS supported out-of-the-box! :smile: (Other OSes need python3.6+ installed)
MIT License
157 stars 6 forks source link

Feature Request: Avoid autostarting nvim-ghost #45

Closed niveK77pur closed 1 year ago

niveK77pur commented 1 year ago

Hello again, please allow me to open 2 issues at once because this plugin seems extremely neat and seems to just work:tm: without too much effort :sweat_smile:

The issue

Upon installing the plugin, it seems to start the nvim-ghost server automatically. On the other hand, with the vim-ghost plugin, you had to explicitly start the server with a command :GhostStart.

While this is definitely very comfortable, I very rarely need GhostText, so having nvim-ghost launch itself every time I open neovim does not feel very ideal to me; I end up disabling the plugin all together.

Therefore, it would be nice to have the ability to either autostart nvim-ghost, or to explicitly start it using some sort of command (like :GhostStart).

At this point in time, I wasn't able to spot any mentions of this (i.e. not autostarting the plugin) being possible.

My installation using packer:

    use { 'subnut/nvim-ghost.nvim', disable=false, -- {{{
        run = ':call nvim_ghost#installer#install()',
    } -- }}}

Food for thought

It might even be interesting to allow autostarting based on autocommand events (i.e. when a certain filename is read, similar to your example in the readme). I suppose autostarting using autocommands would be managed by the plugin users, and will simply call whatever command allows to start the server. Glancing at how the plugin links the text box and neovim, I am not too sure what kind of events could come in question for this.

subnut commented 1 year ago

so having nvim-ghost launch itself every time I open neovim

nvim-ghost doesn't launch every time neovim is opened.
It's launched only once - when neovim is started for the first time.

When neovim is started for the first time after starting the OS, nvim-ghost launches a background process (the python binary), which keeps running even after neovim is closed. This background process orchestrates everything, which keeps the experience seamless.

I don't think it will be possible to have an explicit command to start the server and also keep everything working as seamless as it currently is. I had tried to do that when I created this plugin, but was unsuccessful.

subnut commented 1 year ago

nvim-ghost doesn't launch every time neovim is opened. It's launched only once - when neovim is started for the first time.

Keeping the above in mind, do you still feel like autostarting nvim-ghost is necessary?

niveK77pur commented 1 year ago

Thank you for the clarification! It wasn't clear to me that nvim-ghost launches a process in the background that other neovim instances essentially just attach to (if I understand you correctly now). Maybe I missed in in the README, or maybe it wasn't clearly mentioned.

The confusion must have been fueled from the fact that there is a message saying [nvim-ghost] Server running every time I launch vim (I am aware that I can silence this message). But together with your insights, I see now that it doesn't actually launch it every time anew.

With all that, I have 2 points of view now regarding autostarting nvim-ghost.

Regarding the second point: the rest can still work like it currently does; so when you close neovim, the process remains alive in the background. You would merely transfer starting/stopping of the background process to the user, instead of having neovim launch it when opening it the first time. Maybe a user defined setting could control the behaviour here: (i) automatically launch on startup (default), or (ii) launch/stop manually using certain functions/commands/etc

EDIT: Please do not consider this a very urgent request though! Now that I understand how it works, I feel much more at peace. It would just be the nice little polish that could be nice to implement for the sake of completeness! Just some thoughts~

subnut commented 1 year ago

Initial implementaion - https://github.com/subnut/nvim-ghost.nvim#start-manually

Please test and let me know of any unexpected behaviour.

subnut commented 1 year ago

If you want, I shall add an explanation of what's happening in the initial implementation mentioned above.

niveK77pur commented 1 year ago

Thank you very much for the implementation! Let me tell you what I have tested, what I have found, and what I had expected.

I use the following command to observe the nvim-ghost processes that are running in the background.

watch -n 0.1 pgrep -a ghost

Default auto starting behaviour

Everything works as expected here, keeping in mind the clarifications you provided earlier in this issue.

Enabling the start manually feature

Initial launch of nvim

Adding let g:nvim_ghost_disabled = 1 or vim.g.nvim_ghost_disabled = 1 (depending on VimL or Lua) has the following immediate effects:

Manually starting nvim-ghost

Now, running :GhostTextEnable will cause the following:

Everything works as expected thus far!

Closing nvim after manual launch

Since I manually launched the nvim-ghost process, I was under the impression that the process should exit once I leave nvim as well.

However, it might be annoying to have the process be taken down every time (?), so maybe an additional command such as :GhostTextDisable would make sense — Since I enabled it myself, I should also clean up after and disable it myself.

I am a little split on how to handle this, but I think automatically terminating the process in one way or another would make sense, mainly because it was enabled manually, meaning I don't want to keep it around all the time.

Launching nvim again

Here is where the behaviour starts to diverge from my expectations.

Since the nvim-ghost processes are still running after exiting nvim, I would assume that the plugin catches on to that and attaches to it upon opening nvim. After opening nvim now, I cannot yet use GhostText from the browser.

The unexpected behaviour here is that the nvim-ghost process is running, but the browser cannot connect to the editor.

Manually starting nvim-ghost again

Similar to the auto starting, running :GhostTextEnable will make 2 new nvim-ghost processes appear, which quickly vanish again.

Now GhostText in the browser can connect to the editor.

This works as expected

Conclusion from preliminary tests

Everything seems to behave as expected. The only problem lies in that the plugin does not seem to catch on to an existing nvim-ghost process.

However, seeing that the auto starting behaviour also launches new processes every time I start nvim, makes me suspect that the plugin does not catch onto running processes in this case either. Based on what I see in the pgrep output, the plugin just happens to see it because the new processes exited immediately. I may be completely wrong on this here of course!

With that said; here is my verdict.

  1. Not starting nvim-ghost every time I start nvim seems to work great!
  2. Because of this non-automatic behaviour, I would somehow expect the nvim-ghost process to also terminate after exiting vim. Here, I have two more thoughts.
    • Another :GhostTextDisable command would do the trick! It will be easy enough to add an autocommand myself into the config, which would run this upon exiting vim (or I can run this whenever I am done with GhostText).
    • One could add another option which tells the plugin to auto-terminate the nvim-ghost process upon exiting (in case start manually feature is enabled). That would avoid adding the autocommand on the user's end. A :GhostTextDisable command would still be handy, in case this auto-terminate is disabled.

Food for thought: In case my suspicion, about new processes being launched and exiting right away because another one is already there, is true. Wouldn't it be cool if the plugin connects to the still running nvim-ghost process instead of having to run :GhostTextEnable again? And if no process is running, well then I will have to manually start it. I imagine this could also be interesting for the autostarting behaviour since it avoids launching 2 processes every time nvim is launched. Maybe by storing the PIDs one could check for it more efficiently. I don't know how feasible it is to implement this though, and if it still fits in the scope of this issue. At any rate, I would really only consider this food for thought in particular as a "for fun" improvement since the 2 new processes clearly do not cause any problems (I didn't even know about them until making my tests here).

Edit: Looking through the earlier messages in this issue, it appears that no new processes are being spawned after the first time (in case of the auto starting behaviour, and as a result also in the case of starting manually I suppose). So I am unsure what these two processes are that I see popping up. Apologies if I got something wrong here again!

subnut commented 1 year ago

two new processes being launched and exiting right away because another one is already there

BUG. Fixed in v0.2.3

niveK77pur commented 1 year ago

Attaching to the running nvim-ghost process does not seem to work properly anymore (I hope it is not a silly issue in my config). I observe the problem in both automatic and manual starting.

Assuming automatic starting; when launching nvim for the first time, the 2 nvim-ghost processes are being started. I can launch GhostText in the browser, and everything will be synced in nvim.

Upon closing nvim, and reopening it, the "new processes being launched" bug seems to be gone now! However, once inside of nvim (after the first time), the syncing with GhostText in the browser does not work.

Interestingly, the browser does not complain about not finding the editor, and when I un-focus the browser window, the selected text area has an orange outline (as opposed to blue if it is synced properly).

Not sure if this is a bug that was introduced by accident or not. If you cannot reproduce it, I can try to isolate the issue on my side.

subnut commented 1 year ago

No no, this isn't an issue in your config. This is an actual bug in this plugin.

I'll look into it soon.

subnut commented 1 year ago

Syncing should work now (3a4e443)

subnut commented 1 year ago

Default auto starting behaviour

  • When opening nvim and no processes are running, I will see 2 processes being spawned. They stick around after exiting nvim.
  • When I open nvim again, 2 new processes appear but quickly vanish again (I suspect they detected an already running instance and then exited).
  • Launching GhostText from the browser (assuming nvim is open) will directly sync a text field with nvim.

Everything works as expected here, keeping in mind the clarifications you provided earlier in this issue.

The processes shouldn't spawn anymore. They weren't expected to spawn. They were being spawned due to a bug.

Enabling the start manually feature

Initial launch of nvim

Adding let g:nvim_ghost_disabled = 1 or vim.g.nvim_ghost_disabled = 1 (depending on VimL or Lua) has the following immediate effects:

  • No nvim-ghost processes will be spawned upon opening nvim (assuming none are running yet)
  • Launching GhostText from the browser will have it complain that it cannot connect to the editor (which makes sense, since nvim-ghost is not running)

Manually starting nvim-ghost

Now, running :GhostTextEnable will cause the following:

  • 2 new nvim-ghost processes appear
  • Launching GhostText from the browser will sync the text field with vim

Everything works as expected thus far!

Can confirm, everything is as expected.

Closing nvim after manual launch

Since I manually launched the nvim-ghost process, I was under the impression that the process should exit once I leave nvim as well.

However, it might be annoying to have the process be taken down every time (?), so maybe an additional command such as :GhostTextDisable would make sense — Since I enabled it myself, I should also clean up after and disable it myself.

I am a little split on how to handle this, but I think automatically terminating the process in one way or another would make sense, mainly because it was enabled manually, meaning I don't want to keep it around all the time.

Launching nvim again

Here is where the behaviour starts to diverge from my expectations.

Since the nvim-ghost processes are still running after exiting nvim, I would assume that the plugin catches on to that and attaches to it upon opening nvim. After opening nvim now, I cannot yet use GhostText from the browser.

The unexpected behaviour here is that the nvim-ghost process is running, but the browser cannot connect to the editor.

Manually starting nvim-ghost again

Similar to the auto starting, running :GhostTextEnable will make 2 new nvim-ghost processes appear, which quickly vanish again.

Now GhostText in the browser can connect to the editor.

This works as expected

Conclusion from preliminary tests

Everything seems to behave as expected. The only problem lies in that the plugin does not seem to catch on to an existing nvim-ghost process.

I'll come back to this later (in a future comment).

However, seeing that the auto starting behaviour also launches new processes every time I start nvim, makes me suspect that the plugin does not catch onto running processes in this case either. Based on what I see in the pgrep output, the plugin just happens to see it because the new processes exited immediately. I may be completely wrong on this here of course!

Food for thought: In case my suspicion, about new processes being launched and exiting right away because another one is already there, is true. Wouldn't it be cool if the plugin connects to the still running nvim-ghost process instead of having to run :GhostTextEnable again? And if no process is running, well then I will have to manually start it. I imagine this could also be interesting for the autostarting behaviour since it avoids launching 2 processes every time nvim is launched. Maybe by storing the PIDs one could check for it more efficiently. I don't know how feasible it is to implement this though, and if it still fits in the scope of this issue. At any rate, I would really only consider this food for thought in particular as a "for fun" improvement since the 2 new processes clearly do not cause any problems (I didn't even know about them until making my tests here).

Should be fixed by now. See my comment above (The processes shouldn't spawn anymore. They weren't expected to spawn. They were being spawned due to a bug.).

With that said; here is my verdict.

  1. Not starting nvim-ghost every time I start nvim seems to work great!
  2. Because of this non-automatic behaviour, I would somehow expect the nvim-ghost process to also terminate after exiting vim. Here, I have two more thoughts.
    • Another :GhostTextDisable command would do the trick! It will be easy enough to add an autocommand myself into the config, which would run this upon exiting vim (or I can run this whenever I am done with GhostText).
    • One could add another option which tells the plugin to auto-terminate the nvim-ghost process upon exiting (in case start manually feature is enabled). That would avoid adding the autocommand on the user's end. A :GhostTextDisable command would still be handy, in case this auto-terminate is disabled.

Will comment on this later (in a future comment) (i'm sleepy right now)


Edit: Looking through the earlier messages in this issue, it appears that no new processes are being spawned after the first time (in case of the auto starting behaviour, and as a result also in the case of starting manually I suppose). So I am unsure what these two processes are that I see popping up. Apologies if I got something wrong here again!

Actually, I forgot to mention this as a technicality, but back then, atleast one extra process was being spawned. In order to increase efficiency, just as you mentioned, I moved some of the code to neovim, which was the root cause of the bug that I mentioned in my last comment (the bug that stopped syncing altogether). :sweat_smile:

subnut commented 1 year ago

BTW, it's nice to see such detailed reports from a user. Keep up the good work!

(You're the first person I've seen who's submitted such details. Thanks a lot, again.)

niveK77pur commented 1 year ago

Syncing should work now (3a4e443)

Syncing from the browser now reports that the editor has "disconnected" immediately.

To make sure it was nothing on my side, I completely uninstalled and re-installed the plugin but the problem persists.

Also, now I observe 4 processes being spawned instead of 2. Not sure if the issue could be related :sweat_smile: Though, I would suspect that the call chanclose(l:connection) is misplaced and causes the channel to be closed too soon (I only looked at the commit you highlighted).


And thank you as well! I'm always unsure if I come off as rude or anything because of the long reports, but at the same time I can imagine that more information is always helpful. After all, such a detailed report is my best effort in helping as much as I can!

subnut commented 1 year ago

Phew... finally. Fixed.

Sorry, this has been a hectic week.

Now that the plugin is back to working normally, I can look into this (auto-starting) issue better.

subnut commented 1 year ago

Oops, wrong issue ⬆️

niveK77pur commented 1 year ago

Awesome! The syncing works great now! (At least with the start manually method).

Only other issue I still notice is that when I close the [Scratch] buffer in neovim that gets opened after syncing with the browser, the focused text area does not get deselected (i.e. the blue outline is still there).

After I type something in the still focused text area in the browser, it will report that the editor has disconnected (which makes sense, since I closed the corresponding buffer).

I only tried this with the start manually feature for now as I suspect it is using the exact same code that the auto-starting behaviour under the hood! (Also now I see 5 nvim-ghost processes being spawned in case the issue might be related)


And if you got a notification for a comment that does not exist, I just deleted it because my browser was bamboozling me, sorry for the confusion

subnut commented 1 year ago

Only other issue I still notice is that when I close the [Scratch] buffer in neovim that gets opened after syncing with the browser, the focused text area does not get deselected (i.e. the blue outline is still there).

Bug, fixed in v0.3.2

niveK77pur commented 1 year ago

I hope I am not doing anything wrong, but the problem does not seem to be resolved on my end. Closing the [Scratch] buffer in neovim still does not deselect the text area in the browser. The behaviour is still the same as I described before 🤔 (Also I just noticed, the text area does not get deselected even after closing neovim alltogether)

niveK77pur commented 1 year ago

Also I noticed, the let g:nvim_ghost_disabled = 1 does not take effect anymore. When I open neovim, I still see the nvim-ghost processes being spawned. (I became suspicious because the :GhostTextEnable command was not available anymore)

The variable seems to be correctly set however, since running :echo g:nvim_ghost_disabled reports 1 (which is exactly what I set it to. (If I remove the corresponding line from my config, it will report 0).

Apologies, this here was a dumb mistake in my config, the manual starting still works perfectly fine!

subnut commented 1 year ago

problem does not seem to be resolved on my end.

Yeah, my bad. I had fixed the bug in v0.3.2, but v0.4.0 reintroduced it due to a mistake in the git command I used to revert v0.2.5 (the commit where we shifted from threading to multiprocessing)

Fixed in v0.4.1

niveK77pur commented 1 year ago

Everything works flawlessly now! At a glance I did not notice anything offending that remains! That said, the initial point in my issue has been addressed and is fully functional!

If there is nothing else left on this front which I am forgetting about, then feel free to finally close this issue!

Thanks a lot for the effort, and I hope I wasn't causing you too much trouble! You rock! o/

subnut commented 1 year ago

Closing nvim after manual launch

Since I manually launched the nvim-ghost process, I was under the impression that the process should exit once I leave nvim as well.

This is the ideal behaviour, yes. But it also creates the possibility of a race condition.

Say, this happens -

This problem wouldn't happen if the binary exits fast, but this is python we're talking about. If I ever re-write this in Golang, this might be a feasible option.

However, it might be annoying to have the process be taken down every time (?), so maybe an additional command such as :GhostTextDisable would make sense — Since I enabled it myself, I should also clean up after and disable it myself.

But then, what if there are two (or more) running instances of neovim that are both connected to GhostText? In that case, what should the :GhostTextDisable command do?

subnut commented 1 year ago

Also, please see this again - https://github.com/subnut/nvim-ghost.nvim#start-manually

The variable name has been changed in https://github.com/subnut/nvim-ghost.nvim/commit/10242d4e6964c38ad052b0cbb45fa500daab2c0d

subnut commented 1 year ago
  • If it causes the server to exit, then running :GhostTextDisable in any one neovim instance causes the rest of them to disconnect too.

BTW this can be done by running :call nvim_ghost#helper#kill_server()

subnut commented 1 year ago

This problem wouldn't happen if the binary exits fast, but this is python we're talking about.

I take that back. Looks like it exits almost instantly! 😲

subnut commented 1 year ago

Try out the auto-exit branch

niveK77pur commented 1 year ago

Also, please see this again - https://github.com/subnut/nvim-ghost.nvim#start-manually

The variable name has been changed in 10242d4

Awesome! The renamed variable and command are much clearer now! However, there appears to be a small issue that creeped in. I get the following error: E184: No such user-defined command: GhostTextEnable.

Looking at the code and the commit, it get the impression that there is one place where you forgot to rename :GhostTextEnable to :GhostTextStart. The following solves the issue for me and things behave as they did previously before the command was renamed!

diff --git a/plugin/nvim_ghost.vim b/plugin/nvim_ghost.vim
index 1c39f90..314c304 100644
--- a/plugin/nvim_ghost.vim
+++ b/plugin/nvim_ghost.vim
@@ -54,7 +54,7 @@ if !g:nvim_ghost_autostart
         \  let g:nvim_ghost_disabled = 0
         \| call nvim_ghost#init()
         \| doau <nomodeline> nvim_ghost UIEnter
-        \| delcommand GhostTextEnable
+        \| delcommand GhostTextStart
   finish
 endif

And I see the concerns around the auto-exit behaviour. I shall give the auto-exit branch a try later hopefully, and I will report back on whether things ultimately behaved as anticipated or not!

subnut commented 1 year ago

However, there appears to be a small issue that creeped in. I get the following error: E184: No such user-defined command: GhostTextEnable

Yeah, this has already been fixed by the auto-exit branch. That's why I suggested you to use that branch :sweat_smile:

Anyways, this has now been merged into the main branch. Do report further issues (if any).

subnut commented 1 year ago

Phew

Quite a journey, wasn't it?
Thanks for sticking with me! Your detailed reports helped me immensely!

niveK77pur commented 1 year ago

Oh I see, my bad for not having tried the auto-exit branch then! But I confirm that everything works wonderfully! It appears that the processes are being killed after each nvim instance has exited (at least each one that ran :GhostTextStart).

It was quite the journey indeed, and I want to thank YOU as well for the time and effort!