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

Add support for syncing only on InsertLeave #169

Open
dkasak opened this issue Mar 21, 2019 · 7 comments
Open

Add support for syncing only on InsertLeave #169

dkasak opened this issue Mar 21, 2019 · 7 comments

Comments

@dkasak
Copy link

dkasak commented Mar 21, 2019

This is a feature request for a switch which would mean "only sync on InsertLeave", as per this reddit comment.

I'm not the OP there, but I'd also love this feature as I'm currently considering switching away from LanguageClient-neovim due to the issues described in that thread.

The ergonomically ideal way of handling diagnostics would be to sync periodically (and not only on InsertLeave, just like now), but only update the diagnostics instantly when diagnostics disappear, delaying appearing diagnostics until a future InsertLeave (or a timer expiring, for people that prefer that). I realize this is a much more involved feature, but I wanted to mention it for documentation purposes, in case anyone gets a bout of inspiration to implement it. This is discussed in autozimu/LanguageClient-neovim#603 (e.g. this comment).

@natebosch
Copy link
Owner

only update the diagnostics instantly when diagnostics disappear, delaying appearing diagnostics until a future InsertLeave

Seems interesting - I'd be inclined to aim for that as an option over delaying the sync process. I'd have to look to see how feasible it might be.

I assume a global option would be best for this?

@natebosch
Copy link
Owner

natebosch commented Apr 2, 2019

The difficulty here became obvious when I started digging in - I can't think of a reasonable way to "diff" the diagnostics from what they were previously. More specifically I can't distinguish between a diagnostic that was moved from one being removed and a new one added. This will come up anytime you add a new line in insert mode - all the diagnostics below that line will look new and so they'd disappear.

I wonder if instead if might be better to exclude diagnostic within some range of the cursor. How often is the distraction caused by diagnostic highlighting on the current line compared to other lines?

natebosch added a commit that referenced this issue Apr 2, 2019
See #169

When typing it is common for there to be diagnostics while the code is
in an incomplete state. Suppress these diagnostics until after leaving
insert mode.

This has the unfortunate side effect of also removing existing
diagnostics from the current line even if they weren't impacted by the
changed text.
@andymass
Copy link
Contributor

andymass commented Apr 2, 2019

Admittedly I am not following all the linked conversations, but is the "diff" issue you mentioned just that you can't keep track of diagnostics when the text is being edited? The "text properties" feature was designed to allow placing line/col locations which are automatically updated during editing. Though it's pretty new.

@natebosch
Copy link
Owner

@dkasak - could you try the branch in #173 and let me know your opinion?

It has some unintended side effects of clearing otherwise unaffected diagnostic highlighting for the current line as you type, but that might not be a big deal in practice? Once you leave insert mode, or are typing on a new line and there is an update to the diagnostics, they'll appear.

If you like the behavior of that branch I can either make it an option, or maybe even the default.

@natebosch
Copy link
Owner

is the "diff" issue you mentioned just that you can't keep track of diagnostics when the text is being edited?

Yeah, if I'm typing and I insert a new line all the diagnostics that were online 5 might now be on line 6, but I can't tell when I get the update that it was a moved diagnostic as opposed to a fresh one. Similar problem if things shift by characters instead of lines.

The "text properties" feature was designed to allow placing line/col locations which are automatically updated during editing. Though it's pretty new.

I'm not aware any feature called "text properties". I see a DiagnosticRelatedInformation which is new from the last time I looked at the spec, but I don't see how that would relate to tracking the identity of a diagnostic across updates.

@andymass
Copy link
Contributor

andymass commented Apr 2, 2019

Sorry, text properties is a feature of vim not lsp. Basically like marks except they track columns and can use highlighting. I'm thinking you'd add the received diagnostics as text properties, and then query their locations from vim when you need to identify them again. The other benefit is highlighting moves with the text. Actually, if you are just interested in line numbers, I think you could already do this with signs.

@natebosch
Copy link
Owner

text properties is a feature of vim

Ah cool - I'll check this out. The new concern would likely be that I'd need to take snapshots of the text property locations at the time I sync, but that may be feasible to do. I'll experiment a bit and see what I can can come up with.

Thanks for pointing this out!

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

No branches or pull requests

3 participants