search

minad / corfu

:desert_island: corfu.el - COmpletion in Region FUnction
GNU General Public License v3.0
1.11k stars 43 forks source link

`flymake-show-diagnostics-at-end-of-line` creates a false end of line #414

Open nehrbash opened 8 months ago

nehrbash commented 8 months ago

image

nehrbash commented 8 months ago

Sorry, I accidentally hit enter before I was done writing the ticket. The diagnostic box must already be present and this only happens with corfu-auto and the cursor also needs to be at the end of the line.

minad commented 8 months ago

Thanks for your report. The Flymake diagnostics at the end of the line are a rather new feature, not yet available on Emacs 29. I will revisit this issue after the ~release~ feature-freeze of Emacs 30. Flymake is available on ELPA, but I tend to not update builtin packages since I rely on their stability.

Regarding the point position computation and overlays at point, I am not entirely sure if we had similar issues before. Maybe some of these computation issues were even fixed in Emacs itself. The fix should be easy at least as long as we special case the Flymake overlay (temporarily hiding it for example, or disabling it when Corfu is active). I am usually not fond of such special casing, but maybe it is possible to find a generic approach.

Maybe this is also Flymake issue, if Flymake unintentionally changes the effective position of point. This could happen if the overlay is located before the point or if the before-string overlay property is used. Otherwise the position computation would not be affected.

mekeor commented 8 months ago

Thanks for your report. I was also about to report it right now. :D

Flymake indeed uses the before-string text-property. @minad, should I report a bug to flymake/emacs, wishing it to use the after-string text-property?

mekeor commented 8 months ago

After replacing before-string with after-string in the referenced code of line, then reevaluating[^1] that defun, fixes the issue for me.

[^1]: Use eval-defun (C-M-x) rather than eval-last-sexp (C-x C-e) since it will force a redefinition of the function definition.

mekeor commented 7 months ago

@minad, replacing before-string with after-string in flymake's code leads to another problem: when the flymake-error ranges until the end of a line, then the diagnostics is shown on the next line. thus, i don't think for flymake it's possible to use after-string. what do you think?

minad commented 7 months ago

Is there a way to prevent Flymake from showing the diagnostics during completion? We do the same for Eldoc already:

https://github.com/minad/corfu/blob/d82bc357c9baccfaaca288526d128c895af61274/corfu.el#L1379-L1380

minad commented 1 month ago

@mekeor

replacing before-string with after-string in flymake's code leads to another problem: when the flymake-error ranges until the end of a line, then the diagnostics is shown on the next line. thus, i don't think for flymake it's possible to use after-string. what do you think?

I wonder if this can be corrected if the overlay expands just right before the newline character. Then the error string should not wrap around to the next line. In any case, I think it would make sense to discuss with the new Flymake maintainer Spencer Baugh, or with the contributor who added the Flymake eol diagnostic feature.