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

Initialize neovim clipboard on startup #1

Merged
merged 3 commits into from
Dec 31, 2022
Merged

Initialize neovim clipboard on startup #1

merged 3 commits into from
Dec 31, 2022

Conversation

Jeansidharta
Copy link
Contributor

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
Copy link
Contributor Author

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 astrand/xclip#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
Copy link
Owner

EtiamNullam commented Dec 31, 2022

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
Copy link
Contributor Author

Oh, using getreg works just fine! 🎉 . 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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@Jeansidharta Jeansidharta reopened this Dec 31, 2022
@EtiamNullam EtiamNullam merged commit f86d974 into EtiamNullam:master Dec 31, 2022
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

Successfully merging this pull request may close these issues.

2 participants