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

Support character-wise visual mode word lookup #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

annagrram
Copy link
Contributor

Support visual mode word lookup without overriding the unnamed
register.

Support visual mode word lookup without overriding the unnamed
register.
@beloglazov
Copy link
Owner

I didn't dig deep into it, but with this PR if you invoke the lookup from the visual mode twice, the whole content of the thesaurus buffer gets into the default register. Do you experience this as well?

@annagrram
Copy link
Contributor Author

Very interesting. And weird.
I experience it only when I lookup from visual mode and then lookup regularly.
Lookup twice or more from visual mode works well for me.

I'll dig into it and update the PR.

@annagrram
Copy link
Contributor Author

Ok. I've found and fixed the problem.
Actually it was always there, we just missed it.

Because we use deletions in the Lookup function, it overrides the default register.
I fixed it by saving and restoring the register.

@@ -35,6 +35,7 @@ let s:path = shellescape(expand("<sfile>:p:h") . s:script_name)


function! s:Lookup(word)
let reg_save = @@
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution still caused issues for me. Instead of saving the register here, we can just use the blackhole register on line 50 by replacing it with 1,$d _ and 56 silent! normal! gg5"_dd

@beloglazov
Copy link
Owner

Sorry for taking so long to review your changes. Could you please address my comments? I'll then merge the PR.

@annagrram
Copy link
Contributor Author

Sorry for not fixing it yet. Busy time at work.
I'll take a look at it on the weekend.

@beloglazov
Copy link
Owner

No worries, I have the same issue :)

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