fix(hover): Comment lost first char - #387
Open
geircodes wants to merge 1 commit into
Open
Conversation
…rst character The inline-! fallback regex used a consumed [^!] character class to assert "not a second !", so that character was matched but excluded from the captured group. For a comment with no space right after ! (e.g. "!Name of file to read from"), the leading letter was silently lost. Went unnoticed because the one existing test had a space in that position, and a dropped space is invisible after trim(). Swap the consumed class for lookaround so no comment text is eaten.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(hover): DocCommentReader inline fallback dropped the comment's first character
What happened
Hovering a variable whose declaration has a trailing
!commentwith no space right after the!rendered the comment text with its leading character missing. Example:
Hover showed:
(doc-comment appendix below the location link — missing the leading "N")
Root cause
DocCommentReader.read()'s inline-!-comment fallback used a consumed character class to assert"this isn't a second
!":[^!]matches — and consumes — one character. When that character is the first real letter of thecomment (no space after
!), it's excluded from the captured group and silently lost. The one existingtest for this path (
! Does something useful) has a space in that position, so the dropped characterwas invisible after
.trim()— that's why this went unnoticed.Fix
Replace the consumed character class with lookaround, so no comment text is ever consumed by the
disambiguation check:
(?<!!)— not immediately preceded by another!(so we start at the true beginning of a bang-run)!{1,2}— one or two!(matches the documented intent: single!or!!, not!!!/!!!!)(?!!)— the bang-run doesn't continue into a third!(that's the doc-block marker, alreadyhandled earlier in the function)
(.+)$— captures everything after, untouchedTesting
Two regression tests added to
DocCommentReader.test.ts:bulkfileNamerepro (no space after!) — asserts the full text survives.!!) inline case — asserts the!!-vs-!!!boundary still holds.npm run test:server: all passing, 0 failing (verified in an isolated worktree offorigin/version-1.0.1, no unrelated WIP mixed in).Scope
Two files only:
server/src/utils/DocCommentReader.ts,server/src/test/DocCommentReader.test.ts.Unrelated to the in-flight hover-clickable-locations Issue (
issue-hover-clickable-locations.md) —found incidentally while live-testing that work, but this is a plain correctness bug fix, not a
look-and-feel change, so it goes out as its own PR rather than folded into that Issue+patch.
We should also think about if it is necessary to duplicate this information in the hover.