Skip to content
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

Use different group names for signs and virtual text #4704

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

IngoMeyer441
Copy link
Contributor

Since Neovim commit c4afb9788c4f139eb2e3b7aa4d6a6a20b67ba156, the sign API uses extmarks internally. Virtual text is already rendered using extmarks. ALE uses the same group name for both signs and virtual text and as a result, both are placed in the same extmark group. Since ALE deletes all extmarks in the virtual text group after all signs have been placed, no signs are ever shown. This PR fixes this by renaming the sign group from ale to ale_signs.

Fixes issue #4657.

@IngoMeyer441 IngoMeyer441 force-pushed the fix/ale-signs-in-neovim branch from d1c11e2 to 04ac25e Compare January 8, 2024 16:01
Since Neovim commit c4afb9788c4f139eb2e3b7aa4d6a6a20b67ba156, the sign
API uses extmarks internally. Virtual text is already rendered using
extmarks. ALE uses the same group name for both signs and virtual text
and as a result, both are placed in the same extmark group. Since ALE
deletes all extmarks in the virtual text group after all signs have been
placed, no signs are ever shown. This commit fixes this by renaming the
sign group from `ale` to `ale_signs`.
@IngoMeyer441 IngoMeyer441 force-pushed the fix/ale-signs-in-neovim branch from 04ac25e to a3183e2 Compare January 8, 2024 16:13
@IngoMeyer441
Copy link
Contributor Author

The tests seem to pass now...

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work without issues. Thanks.

@hsanson hsanson merged commit 562680e into dense-analysis:master Jan 14, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants