-
Notifications
You must be signed in to change notification settings - Fork 85
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
Comments
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? |
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? |
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.
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. |
@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. |
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.
I'm not aware any feature called "text properties". I see a |
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. |
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! |
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 futureInsertLeave
(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).The text was updated successfully, but these errors were encountered: