Skip to content

fix notes crashing and leaking single comments notes#4828

Merged
fendor merged 1 commit into
haskell:masterfrom
vidit-od:crashing-notes-plugin
Feb 6, 2026
Merged

fix notes crashing and leaking single comments notes#4828
fendor merged 1 commit into
haskell:masterfrom
vidit-od:crashing-notes-plugin

Conversation

@vidit-od

@vidit-od vidit-od commented Feb 5, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes :

  1. Issue hls-notes-plugin crashes instead of reporting no locations #4763
  2. Leaking Single Comment Notes Declaration
singleLine Leak

here the definatination declaration is :
singleLine Leak Def

@vidit-od
vidit-od requested a review from jvanbruegge as a code owner February 5, 2026 08:53
@vidit-od
vidit-od force-pushed the crashing-notes-plugin branch from aaafb32 to 27fb06c Compare February 5, 2026 08:59
Comment thread plugins/hls-notes-plugin/src/Ide/Plugin/Notes.hs
@fendor

fendor commented Feb 5, 2026

Copy link
Copy Markdown
Collaborator

Leaking Single Comment Notes Declaration

What does this mean?

@vidit-od

vidit-od commented Feb 5, 2026

Copy link
Copy Markdown
Collaborator Author

My bad, i did not do a good job explaining the issue :

There are 2 ways to do Note Declaration
1 ) multi line comments ( or Block Note)
2 ) single line comments ( or Line Note ) Example below :

singleLine Leak Def

The Note ending condition for 2nd type of declaration was not defined properly. Hence the content outside it was also picked up as content. Below image has {- Note [Not a valid note] which should not be there in hover.

singleLine Leak

To Prevent this we have to Define proper ending conditions, which are different for both the types on Notes. Hence in the above implementation we first check the type of note declaration and extract content accordingly !

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you!

Waiting for @jvanbruegge to review as well

Comment thread plugins/hls-notes-plugin/src/Ide/Plugin/Notes.hs Outdated

@jvanbruegge jvanbruegge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Aside from @fendor's small nit LGTM

@vidit-od
vidit-od force-pushed the crashing-notes-plugin branch from 27fb06c to c01495b Compare February 6, 2026 10:20
@vidit-od

vidit-od commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator Author

Done !!
Thank you for reviewing this PR.

@fendor
fendor enabled auto-merge (squash) February 6, 2026 18:01
@fendor
fendor merged commit 09a87da into haskell:master Feb 6, 2026
53 of 57 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.

3 participants