Skip to content

feat: clear lsp references on ui open#30

Open
A-Lamia wants to merge 2 commits into
cshuaimin:mainfrom
A-Lamia:clear-lsp-references
Open

feat: clear lsp references on ui open#30
A-Lamia wants to merge 2 commits into
cshuaimin:mainfrom
A-Lamia:clear-lsp-references

Conversation

@A-Lamia

@A-Lamia A-Lamia commented Aug 10, 2023

Copy link
Copy Markdown
Contributor

If you're hovering over text with a LSP, it highlights all the references, when entering SSR these highlights persist, this can be visually noisy

Before:

before.mp4

After:

after.mp4

@cshuaimin

Copy link
Copy Markdown
Owner

Thanks for bringing this problem to me!

I wonder if it is more useful when you clear lsp highlights in an BufLeave autocmd? And you can use vim.lsp.buf.clear_references().

@cshuaimin

Copy link
Copy Markdown
Owner

I found related code in AstroNvim, maybe we can also add BufLeave event to AstroNvim, to clear lsp highlights when you enter another buffer (a vsplit or :h).

@A-Lamia

A-Lamia commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

Hmmm, yes, i'm currently using my own autocmd for this, but you might be right this probably is not needed in ssr, if people don't have that function implemented them selves they will not have this "side effect" I'll have this solved in astro.

:)

@A-Lamia A-Lamia closed this Aug 11, 2023
@cshuaimin

Copy link
Copy Markdown
Owner

No I think it's useful to clear lsp highlights in ssr.nvim. If you change the PR to use vim.lsp.buf.clear_references() in an autocmd, then I'm happy to merge this.

@A-Lamia A-Lamia reopened this Aug 11, 2023
@A-Lamia

A-Lamia commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

I've found a pretty bad bug that is the root of some issues, ill be doing some testing and ill get back to you once i figure everything vim.lsp.buf.clear_references() might not be the best use case for this situation.

EDIT: an autocmd also may not be the best choice, autocmd is typicality used when you want to automate a function execution, but in our case with SSR we are always executing Ui:open() manually, the Autocmd is just extra overhead for the same result, if that is what you want however i have no issue with doing it via autocmd.

@A-Lamia

A-Lamia commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

aah something i didn't think of, if people leave ssr active while editing code in this case yeah the autocmd makes sense, this was an oversight on my part as i typically don't use ssr like that.

SSR creates instances when you Ui:Open but the autocmds are not treated
like an instance they are sharing the same autocmds, these autocmds how
ever are being duplicated on each run of Ui:Open causing bad side effects
like data invalidation changes in this commit creates a unique augroup
and allows only one autocmd per instance then removes them once the
instance is closed.
@A-Lamia A-Lamia force-pushed the clear-lsp-references branch from e38f2cf to 7602055 Compare August 12, 2023 07:42
@A-Lamia

A-Lamia commented Aug 12, 2023

Copy link
Copy Markdown
Contributor Author

need to merge #32 before this PR.

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