EtiamNullam / deferred-clipboard.nvim

Keep clipboard in sync with Neovim without the peformance hit.
73 stars 2 forks source link

Initialize neovim clipboard on startup #1

Closed Jeansidharta closed 1 year ago

Jeansidharta commented 1 year ago

Hi!

I noticed a small issue: the internal neovim register " is not initialized on startup, making a focus switch necessary to make sure it's properly set. This pull request should fix that.

Loved the project's approach. Let me know if this is not appropriate, or if changes are needed :)

Jeansidharta commented 1 year ago

After making the proposed changes, I started encountering this error randomly when opening neovim:

clipboard: error: Error: target STRING not available

Apparently, this is related to https://github.com/astrand/xclip/issues/38 and the fact that I'm using xclip. One way I found to still implement this change, and not get this error, is to do something like this on the setup function:

vim.defer_fn(function () copy_register('+', '"') end, 1000)

Which is just a way to wait a bit before setting the register. But since this is not guaranteed to work, and is very hacky, I'll be closing this pull request until I have a better solution

EtiamNullam commented 1 year ago

Thank you, I'm glad you like it!

That's a valid issue and your solution seems to work for me (wink32yank on Windows here), but I feel like it should be optional, as we cannot assume that setup() will always be called directly at launch, so I will make it optional after the merge. It could potentially replace something important in the unnamed buffer with the content of system's clipboard.

Is the defer_fn way solving the problem with the error? Is it also working correctly with vim.schedule? It seems like it's adding significant overhead at startup this way, so it would be good to delay it somehow anyway.

EDIT: I believe it would be fine if we simply check if unnamed buffer is empty and only then replace it with clipboard content.

Is the error also displayed if you replace getreginfo with getreg in copy_register function?

Jeansidharta commented 1 year ago

Oh, using getreg works just fine! :tada: . I have no idea why.

It did not work using schedule instead of defer_fn, unfortunately :(

This seems to be an issue exclusively with xclip. Other clipboard managers seem to work just fine either way.

You think it'd be okay to replace getreginfo with getreg? If so, I can reopen this pull request with the needed change.

Also, I think you're right that we should not assume setup() is called at launch. And I also like your idea of verifying if the unnamed register is empty or not. But I can't think of an easy and simple way of implementing both in a config.

EtiamNullam commented 1 year ago

I believe that I had some issues with getreg before, but feel free to proceed: apply it and reopen the PR. I will test it before releasing.

I feel closing it was not needed, I will not blindly accept it without checking 😉

Are you sure this error does not appear in your :messages even if you delay it (while using getreginfo)?

Jeansidharta commented 1 year ago

Oh yes, you're right, the error still appears in my messages with the 1 second delay. I didn't realize that. This is what I see when running nvim and waiting one second:

screenshot

If no delay is set, this is what shows in my terminal, 50% of the time I try to open vim. It just kills the process.

image

Jeansidharta commented 1 year ago

I've submitted the change from getreginfo to getreg, as you said. Though this will still be executed at startup, with no configuration