-
Notifications
You must be signed in to change notification settings - Fork 3
Make emulated grep support --color argument
#83
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
Conversation
--color arg--color argument
internal/grep/grep_test.go
Outdated
| pattern: "\\d", | ||
| input: "a1b", | ||
| colorMode: "always", | ||
| expected: Result{ExitCode: 0, Stdout: []byte("a" + utils.HighlightString("1") + "b")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to avoid logic in assertions - hardcode the expected bytes here. That way we're protected against accidental changes to the logic inside utils.HighlightString.
…sion in highlighting_test_case.go
Paul formatting fixes
Add multiple lines stage in highlighting extension (3/3)
…xtures Record fixtures for highlighting assertion (2/3)
Highlighting Assertion (1/3)
Launch grep inside PTY
| if r.hasCursor() { | ||
| // If the cursor is on the line, we need to preserve the spaces before the cursor | ||
| contentsBeforeCursor := rawCellContents[:r.cursorCellIndex] | ||
| contentsAfterCursor := strings.TrimRight(rawCellContents[r.cursorCellIndex:], " ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Cell index used as byte offset for string slicing
The getTextContents() function uses r.cursorCellIndex as a byte offset when slicing rawCellContents, but cursorCellIndex is a cell/column index, not a byte position. Since rawCellContents is built by concatenating cell.Content values (which can contain multi-byte Unicode characters), using the cell index as a byte offset produces incorrect slices or invalid UTF-8 strings when processing text with non-ASCII characters.
|
|
||
| for _, match := range matchedStrings { | ||
| matchIdx := strings.Index(remaining, match) | ||
| beforeMatch := remaining[:matchIdx] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing bounds check causes panic when match not found
The highlightMatches function uses strings.Index(remaining, match) to find match positions but doesn't handle the case where it returns -1. If matchIdx is -1, the expression remaining[:matchIdx] becomes remaining[:-1] which is invalid in Go and causes a panic. While the matchedStrings come from regex matches that should exist in the line, there's no defensive check for edge cases like unexpected matcher behavior or overlapping matches.
Note
Adds --color highlighting (always/never/auto with TTY) to emulated grep and introduces VT-based VT/ANSI assertions, stages, and fixtures to validate highlighting behavior.
--colorsupport (always|never|auto) with TTY-aware behavior viaEmulationOptions.-obehavior unchanged.HighlightingAssertionwith VT-based comparison of text and ANSI styles (bold red only), plus detailed mismatch logs.virtual_terminalpackage (VT emulator, screen state, CRLF translation).bm2,eq0,wg2,jk4,na5with fixtures and failure scenarios; integrate intostages_test.goand Makefile targettest_highlighting_with_grep.HighlightStringandColorModeutilities.ultraviolet,x/vt, ANSI-related deps; updatetester-utilsand others.OrderedLinesAssertion.Written by Cursor Bugbot for commit 777260c. This will update automatically on new commits. Configure here.