After fixing an error and save buffer, lsp-ui still displays the error message

[hisnawi]

I am enabling flycheck syntax checking only on save with :

 flycheck-check-syntax-automatically  '(mode-enabled save)

And disabling lsp ui live reporting:

      (setq lsp-ui-flycheck-live-reporting nil)

however, if I follow the steps:
edit a line and cause an error -> save buffer -> lsp-ui shows error message -> fix error -> save buffer -> lsp-ui still shows same error message

If I check flycheck list of errors the error is not listed there anymore which tells me it is a lsp-ui issue.

[melton1968]

I was able to work around this issue with the following one line patch to lsp-ui-flycheck.el:

diff --git a/lsp-ui-flycheck.el b/lsp-ui-flycheck.el
index 3c48710..c5d5cb4 100644
--- a/lsp-ui-flycheck.el
+++ b/lsp-ui-flycheck.el
@@ -216,7 +216,6 @@ See https://github.com/emacs-lsp/lsp-mode."

 (defun lsp-ui-flycheck--report nil
   (and flycheck-mode
-       lsp-ui-flycheck-live-reporting
        (flycheck-buffer)))

 ;; FIXME: Provide a way to disable lsp-ui-flycheck

I am unfamiliar with the code-base, so I am uncertain if this is the correct fix, but it is working for me with the configuration listed above. Without this change, there is no way that flycheck can keep the errors messages in sync with the last result from the language server.

If you have installed lsp-ui from elpa, you can simply delete the line from the file and remove the corresponding .elc file and then restart emacs.

[hisnawi]

Hmm. it dosen't seem that worked for me.
I removed the line and reloaded emacs, but I still see live updates by flycheck.

[melton1968]

After a little more testing, I see that in general you are correct. At this point, this is what I observe: At global scope, I get live error reporting. In inner scopes (e.g. object abc { … }), I get the desired behavior. So, the fix I suggested is clearly not correct, although it does makes the syntax checking much more usable for me since I am typing in some inner scope much more often that at global scope. I will continue with it until the real fix is in place. ps. I presume you removed the .elc file?

…

On Dec 28, 2019, at 11:21 PM, hisnawi ***@***.***> wrote: Hmm. it dosen't seem that worked for me. I removed the line and reloaded emacs, but I still see live updates by flycheck. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#347?email_source=notifications&email_token=AAHONJA7XINM3OWLSHBEAL3Q3AQSZA5CNFSM4JVBGZJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHYXSFA#issuecomment-569473300>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHONJC3M75EVXQBWPKUGE3Q3AQSZANCNFSM4JVBGZJA>.

[hisnawi]

There was no .elc file. Did you mean in the elpa/lsp-ui(version-number) directory?

[melton1968]

Yes, here is what I see in my emacs.d directory for lsp-ui (MacOSX): $ ls -al .emacs.d/elpa/lsp-ui-20191228.1151/ total 496 drwxr-xr-x 17 mmelton staff 544 Dec 28 22:33 . drwxr-xr-x 29 mmelton staff 928 Dec 28 09:07 .. -rw-r--r-- 1 mmelton staff 2220 Dec 28 09:06 lsp-ui-autoloads.el -rw-r--r-- 1 mmelton staff 30760 Dec 28 09:06 lsp-ui-doc.el -rw-r--r-- 1 mmelton staff 28389 Dec 28 09:06 lsp-ui-doc.elc -rw-r--r-- 1 mmelton staff 1504 Dec 28 09:06 lsp-ui-doc.html -rw-r--r-- 1 mmelton staff 9335 Dec 28 22:31 lsp-ui-flycheck.el -rw-r--r-- 1 mmelton staff 12616 Dec 28 09:06 lsp-ui-imenu.el -rw-r--r-- 1 mmelton staff 11405 Dec 28 09:06 lsp-ui-imenu.elc -rw-r--r-- 1 mmelton staff 29922 Dec 28 09:06 lsp-ui-peek.el -rw-r--r-- 1 mmelton staff 28452 Dec 28 09:06 lsp-ui-peek.elc -rw-r--r-- 1 mmelton staff 482 Dec 28 09:06 lsp-ui-pkg.el -rw-r--r-- 1 mmelton staff 24396 Dec 28 09:06 lsp-ui-sideline.el -rw-r--r-- 1 mmelton staff 21438 Dec 28 09:06 lsp-ui-sideline.elc -rw-r--r-- 1 mmelton staff 6105 Dec 28 09:06 lsp-ui.el -rw-r--r-- 1 mmelton staff 5709 Dec 28 09:06 lsp-ui.elc I manually removed the lsp-ui-flycheck.elc file and didn’t bother to re-compile it.

…

On Dec 29, 2019, at 12:05 AM, hisnawi ***@***.***> wrote: There was no .elc file. Did you mean in the elpa/lsp-ui(version-number) directory? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#347?email_source=notifications&email_token=AAHONJDTUQM3AR7PVK4PCODQ3AV2RA5CNFSM4JVBGZJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHYYCSY#issuecomment-569475403>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHONJHHL2UYT5NXM4UNBQDQ3AV2RANCNFSM4JVBGZJA>.

[hisnawi]

Not for me. No .elc files.

[melton1968]

After poking around the code a bit, here is my current understanding:

Diagnostic reports are received from the language server asynchronously and notifications can be received by using the 'lsp-after-diagnostics-hook. We would like to update the fly-check errors only when the report from the language server was induced by saving the buffer. As far as I can tell, there is no direct way to determine this.

Trying to use 'after-save-hook creates a race condition between 'after-save-hook and 'lsp-after-diagnostics-hook.

As an improved work-around for the time being, I made the following changes to lsp-ui-flycheck.el which uses the simple heuristic that if a diagnostics report arrives and the buffer is unmodified, then call flycheck-buffer.

Seems to work reasonably well.

diff --git a/lsp-ui-flycheck.el b/lsp-ui-flycheck.el
index 3c48710..eb6d91e 100644
--- a/lsp-ui-flycheck.el
+++ b/lsp-ui-flycheck.el
@@ -216,14 +216,14 @@ See https://github.com/emacs-lsp/lsp-mode."

 (defun lsp-ui-flycheck--report nil
   (and flycheck-mode
-       lsp-ui-flycheck-live-reporting
+       (or lsp-ui-flycheck-live-reporting
+	   (not (buffer-modified-p)))
        (flycheck-buffer)))

 ;; FIXME: Provide a way to disable lsp-ui-flycheck
 (defun lsp-ui-flycheck-enable (_)
   "Enable flycheck integration for the current buffer."
-  (when lsp-ui-flycheck-live-reporting
-    (setq-local flycheck-check-syntax-automatically nil))
+  (setq-local flycheck-check-syntax-automatically nil)
   (setq-local flycheck-checker 'lsp-ui)
   (lsp-ui-flycheck-add-mode major-mode)
   (add-to-list 'flycheck-checkers 'lsp-ui)

[yyoncho]

@melton1968 I was about to propose the same workaround. Would you like to create a PR?

(not (buffer-modified-p))

Here you should also check whether the flycheck mode is on saved because if it is manual the errors should be rendered only when the user calls flycheck-buffer.

[melton1968]

Yes, the current patch does not respect the value of flycheck-check-syntax-automatically and in fact sets it to nil to keep flycheck from checking the buffer before the diagnostics are received. I can see how to support the following configurations with minimal code changes: 1. lsp-ui-flycheck-live-reporting is not nil lsp-ui-flycheck—report invokes (flycheck-buffer) every time new diagnostics are received 2. lsp-ui-flycheck-live-reporting is nil and flycheck-check-syntax-automatically contains ’save lsp-ui-flycheck—report invokes (flycheck-buffer) when new diagnostics are received and the buffer is unmodified 3. lsp-ui-flycheck-live-reporting is nil and flycheck-check-syntax-automatically does not contain ’save lsp-ui-flycheck—report does not invoke (flycheck-buffer) If that makes sense, I will make the changes and submit a PR.

…

On Dec 29, 2019, at 2:48 PM, Ivan Yonchovski ***@***.***> wrote: @melton1968 <https://github.com/melton1968> I was about to propose the same workaround. Would you like to create a PR? (not (buffer-modified-p)) Here you should also check whether the flycheck mode is on saved because if it is manual the errors should be rendered only when the user calls flycheck-buffer. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#347?email_source=notifications&email_token=AAHONJBGWTQ5R5AKLBOL6BDQ3D5ITA5CNFSM4JVBGZJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZG5JQ#issuecomment-569536166>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHONJFBT7LBIAIAQT2TCUTQ3D5ITANCNFSM4JVBGZJA>.

[yyoncho]

If that makes sense, I will make the changes and submit a PR.

It makes sense.

[hisnawi]

@melton1968 thank you for looking into this

[deviantfero]

Hi, I'm still seeing this issue, has this changed reached the melpa repository yet?