Fix rename type assertion, find regex handling, and scroll comments#7
Merged
Conversation
- Remove unnecessary (this as any) type assertion in rename() — label is already declared as a public property - Add try/catch around RegExp construction in find() to gracefully handle invalid regex patterns instead of throwing - Remove dead code in wait() — the listener filter `typeof l === "function"` was always true since all listeners are functions; notifyListeners() already clears the array - Fix scroll direction comments to match xterm.js scrollLines() semantics (positive=down/newer, negative=up/older) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
|
Nice, clean fixes — each one is exactly right. Thanks for taking the time to read the code carefully. |
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
2 participants
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.
Summary
(this as any)type assertion inrename()—labelis already a public class property, no type cast neededtry/catcharoundRegExpconstruction infind()to gracefully return empty matches for invalid regex patterns instead of throwing an unhandled errorwait()— the listener filtertypeof l === "function"was always true since all listeners are functions;notifyListeners()already clears the array on each notificationscrollLines()semantics: positive = scroll down (view newer content), negative = scroll up (view older content)Test plan
vitest run)tsc --noEmit)🤖 Generated with Claude Code