syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.67k stars 4.89k forks source link

Bad interaction of flyspell and LSP headline breadcrumb #14292

Closed Kazark closed 8 months ago

Kazark commented 3 years ago

Description :octocat:

Flyspell is marking code symbols in the LSP's headerline breadcrumb as mispelled. I don't know whether this is something we can fix in Spacemacs by making the two play nicely, or whether it should be reported upstream; and if upstream, to which project.

Reproduction guide :beetle:

Open a file from a Scala project

Observed behaviour: :eyes: :broken_heart: Squiggly underlines in LSP's headerline breadcrumb.

image

Expected behaviour: :heart: :smile: LSP notifies Flyspell that it is using the headerline and so to please ignore it?

System Info :computer:

Kazark commented 3 years ago

I'm not longer observing this. Perhaps a package update fixed it.

Kazark commented 3 years ago

I saw it again, this time in a Java file.

dj0405 commented 3 years ago

I'm not longer observing this. Perhaps a package update fixed it.

I am observing the same issue with C files!

Kazark commented 3 years ago

Why am I not seeing it in in Scala files now though? So confusing

Kazark commented 3 years ago

Now I am seeing it in one Scala file but not the other! 😮

dj0405 commented 3 years ago

It seems it has to do with the path name! I have two C files open from two different projects. One file shows the spell check underline and one file seems okay! Very confusing!

eklitzke commented 3 years ago

I also see this intermittently. My guess is that flyspell has some heuristic it applies to detect if text is English or not, and if enough of it is English it's shows the spell check errors. In my example A few of the items on the breadcrumb line are valid dictionary words in English, and some aren ot.

eklitzke commented 3 years ago

I have a fix (I think), by adding this to my dotspacemacs/user-config:

  (spacemacs|use-package-add-hook lsp-mode
    :post-config
    (add-hook 'lsp-headerline-breadcrumb-mode-hook 'flyspell-mode-off))

I will see if I can make this work in the develop branch and make a PR.

Kazark commented 3 years ago

@eklitzke Excellent. Thank you!

eklitzke commented 3 years ago

That didn't actually fix it, but now I think that this actually might just be controlled by the variable lsp-headerline-breadcrumb-enable-diagnostics which colorizes the faces of headerline breadcrumb elements based on LSP diagnostics. I set that to nil in my config and the problem isn't happening right now (although it didn't always immediately happen before either).

lebensterben commented 3 years ago

That didn't actually fix it, but now I think that this actually might just be controlled by the variable lsp-headerline-breadcrumb-enable-diagnostics which colorizes the faces of headerline breadcrumb elements based on LSP diagnostics. I set that to nil in my config and the problem isn't happening right now (although it didn't always immediately happen before either).

@eklitzke this is not a fix. it just turns off this feature

jclosure commented 3 years ago

Not Spacemacs-specific. I'm seeing this with vanilla Emacs as well. image

EDIT: This is effect is actually just attributes of lsp-headerline-breadcrumb-path-face. I removed the underline from the face and it looks much better.

Kazark commented 3 years ago

@jclosure if you send me a snippet of your code I'll put it into Spacemacs. Thanks for discovering the solution.

Kazark commented 3 years ago

Or is your change something that should be upstreamed?

jclosure commented 3 years ago

You can just use M-x list-faces-display, then do an isearch (C-s) for lsp-breadcrumb. It will show faces used to display the breadcrumbs. You can click the links in here to customize these faces. Here's mine after turning the underline off for lsp-headerline-breadcrumb-path-face using the customize menu. It's really just cosmetic.

image

Kazark commented 3 years ago

@lebensterben what do you think the right course is here? Just put these changes into the lsp layer? Try to argue upstream at lsp-mode that the defaults do not make sense?

lebensterben commented 3 years ago

@jclosure @Kazark This is not a fix.

The documentation of lsp-headerline-breadcrumb-path-warning-face describes that

Face used for breadcrumb paths on headerline when there is an warning under that path

So having curly underline is correct and idiomatic. Unsetting this property doesn't change the fact that spell-checking is interfering with the headerline.

lebensterben commented 3 years ago

https://github.com/emacs-lsp/lsp-mode/issues/2520

Two workarounds (NOT FIX) provided in this discussion. They either turn off or partially turn off the diagnostics feature in headerline, which is not desirable if you actually want it.

lebensterben commented 3 years ago

I cannot come up with a proper FIX for now.

The minor mode lsp-headerline-breadcrumb-mode works on the variable header-line-format, which is defined in emacs upstream.

So theoretically, though I haven't experienced, the irritating behaviour of flyspell on lsp-headerline could also occur to header line in general. Therefore, I think we'd better ask for help from the broader emacs community about whether there's a proper solution to this issue. If there isn't, then we may need to request a feature that to disable flyspell for headerline!

P.S. I'm currently tied up by some other issues/bugs. If anyone have reported this upstream or sent this to one of emacs' developer mailist, please keep me updated.

Kazark commented 3 years ago

I am not using LSP in most of my development at the moment so this is a low-urgency thing for me personally right now. I may start using it again after a while though; I was using Metals but the monorepo I mostly work in is so big that it was murdering Metals. I am working on subdividing the monorepo so once I turn Metals back on this is likely to annoy me again.

Kazark commented 3 years ago

I see it in Rust mode too.

SeanMaclochlainn commented 2 years ago

I'm on Doom Emacs but was having the same issue. It went away when I installed dictionaries for Hunspell, the dictionary for flyspell

mkdir -p ~/Library/Spelling
wget -O en_US.aff  https://cgit.freedesktop.org/libreoffice/dictionaries/plain/en/en_US.aff\?id\=a4473e06b56bfe35187e302754f6baaa8d75e54f
wget -O en_US.dic https://cgit.freedesktop.org/libreoffice/dictionaries/plain/en/en_US.dic\?id\=a4473e06b56bfe35187e302754f6baaa8d75e54f

I was also getting "Can’t find Hunspell dictionary with a .aff affix file" which also went away

Kazark commented 2 years ago

Thanks @SeanMaclochlainn . I'll give that a shot the next time I hit this.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!

benleis1 commented 6 months ago

I'm seeing that behavior today. Has there been any progress discussing with the lsp folks?