WIP: Lint on FocusGained/VimResume by andreypopp · Pull Request #2494 · dense-analysis/ale

andreypopp · 2019-05-12T10:08:54Z

This PR allows to configure ALE to lint when vim gains focus or is resumed after
being suspended (C-z and then fg command).

The motivation for this is that some linters do not watch all needed resources
(build artifacts) and thus they require to be nudged after those are changed.
The good way I found is to do that when you switch back to an editor.

Example workflow:

Work in vim, do some edits
Switch to terminal and run build process so it does some updates to build
artifacts
Switch back to an editor. This is the point where I want to see updated
diagnostics for the buffer but right now I have to save buffer manually so it
triggers the lint (and didSave event for an LSP). With with PR vim sends
didChange event to LSP / requests lints automatically when I switch to back
to it.

Not that for this to work with tmux one must set

set -g focus-events on

in their tmux.conf.

Note also that I've added b:force_changedtick which forces to send LSP didChange event even if there were no changes.

This commits allow to configure ALE to lint when vim gains focus or is resumed after being suspended (C-z and then `fg` command). Not that for this to work with tmux one must set ``` set -g focus-events on ``` in their `tmux.conf`.

w0rp

Add tests for the new event, and update the tests that check which autocmd commands are set.

w0rp · 2019-05-12T11:21:58Z

autoload/ale/events.vim

 endfunction

+function! ale#events#FocusGained(buffer) abort
+    call setbufvar(a:buffer, 'force_changedtick', 1)


We shouldn't force updates to be sent to language servers here and below. We should only send updates if the buffer has actually changed. If b:changedtick isn't updated because some other process updated the buffer and Vim didn't increment b:changedtick, we can do something that causes b:changedtick to increment in response to the buffer being changed, maybe comparing the text before and after Vim lost focus. If the buffer hasn't been changed, no messages should be sent to the language servers.

This is done so we can request server to recompute diagnostics. Unfortunate LSP lacks such method and I see you’ve also noted that — microsoft/language-server-protocol#737

I understand. I think we shouldn't request diagnostics again if the buffer hasn't changed.

Do you think we can add such feature (requesting diagnostics via sending didChange events) configurable for specific LSP linters? In my case I'm working on ocamlmerlin-lsp which only works per-file — and if I run a build system — I want to re-request diagnostics from the linter no matter if buffer was changed or not (there could be new/no errors just because of the build process finished).

Btw such behaviour can be also useful for non-LSP linters — they don't work with "push" model (I think LSP is designed around this) also, instead you ask / "pull" diagnostics from them.

It won't work for all language servers. Languages servers don't always respond to textDocument/didChange with diagnostics. The Language Server Protocol needs to be changed to support explicitly requesting diagnostics for such a thing to work. The protocol doesn't offer any guarantees about what will trigger diagnostics being sent, when they will arrive, or if they will ever be sent. I think it's one of the flaws of the protocol.

tsserver does have an explicit command for requesting diagnostics and pretty much guarantees a response, so we could add a command to request for diagnostics for tsserver.

Right, I understand that.

That's why I'm asking if we can make this configurable per-linter so it works with ocamlmerlin-lsp (either via textDocument/didChange or I can add some new type of request to query for diagnostics as a protocol extension).

I think we should follow the protocol and only send a message for a document being changed when it has changed, and push for the protocol to be extended to allow user to request diagnostics explicitly, like you can with tsserver.

Unfortunately I don't think I can wait till it's standardised in LSP — standard bodies move slowly. I prefer standards instead be informed by real world usage. I think I'll be able to patch it up on top of ALE, not optimal but doable.

w0rp · 2019-05-12T11:22:53Z

autoload/ale/events.vim


+function! ale#events#FocusGained(buffer) abort
+    call setbufvar(a:buffer, 'force_changedtick', 1)
+    call ale#Queue(0, 'lint_file', a:buffer)


Check g:ale_lint_on_focus_gained here again, and probably ale_enabled like we do above. Otherwise it will be difficult to turn this event off after the autocmd command has been set.

w0rp · 2019-05-12T11:24:05Z

plugin/ale.vim

 let g:ale_lint_on_filetype_changed = get(g:, 'ale_lint_on_filetype_changed', 1)

+" This flag can be set to 1 to enable linting when vim gains focus.
+let g:ale_lint_on_focus_gained = get(g:, 'ale_lint_on_focus_gained', 1)


Document the option and turn it off by default. There are thousands of people using ALE, and many of them will have told ALE not to lint their buffers when they enter a file. They will be surprised if it starts linting their buffers when Vim regains focus after they update ALE.

Hm... maybe we should use an existent check g:ale_lint_on_enter, what do you think? Not sure adding an extra option is worth it considering the semantics of "entering" is so poorly defined.

Also — maybe we can make such behaviour configurable per-linter?

I think a new option is appropriate, but it should be off by default.

w0rp · 2019-05-12T19:32:40Z

You can get support for linting buffers on VimResume or FocusGained in without the changes to the LSP code. You can take that part out and add support for just the new events.

andreypopp · 2019-05-13T06:46:29Z

Yeah, good point.

w0rp · 2020-04-17T16:36:58Z

I'm closing old pull requests that didn't go anywhere, or I didn't have time to look at. Re-open the pull request or submit a new one if you have something you think can be merged.

Lint on FocusGained/VimResume

ebbbeac

This commits allow to configure ALE to lint when vim gains focus or is resumed after being suspended (C-z and then `fg` command). Not that for this to work with tmux one must set ``` set -g focus-events on ``` in their `tmux.conf`.

w0rp requested changes May 12, 2019

View reviewed changes

andreypopp closed this May 12, 2019

andreypopp reopened this May 13, 2019

w0rp closed this Apr 17, 2020

Conversation

andreypopp commented May 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

w0rp left a comment

Choose a reason for hiding this comment

Uh oh!

w0rp May 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0rp commented May 12, 2019

Uh oh!

andreypopp commented May 13, 2019

Uh oh!

w0rp commented Apr 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andreypopp commented May 12, 2019 •

edited

Loading

w0rp May 12, 2019 •

edited

Loading