fix: address bugs in nolint parsing, unicode alignment, and flag stripping#50
Merged
Conversation
…pping - internal/align: refine parseNolint to support block comments with leading stars and multiple lines (e.g., /* * nolint */). - internal/ui: improve truncPad to account for double-width CJK characters, ensuring stable column alignment in side-by-side diffs. - internal/app: refactor egg flag stripping to robustly handle both exact and -flag=value syntax. - internal/match: deduplicate CSV splitting logic into a shared SplitCSV helper. - internal/ui: ensure test predictability by handling nil *os.File in terminal width and color detection.
Refactor the preprocessing of command-line arguments to isolate the retro-theme "easter egg" flag logic into a dedicated stripEggFlags method. This improves the readability of the main Run orchestration while switching the implementation to a more robust and concise regular expression approach.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #50 +/- ##
==========================================
+ Coverage 92.88% 93.15% +0.26%
==========================================
Files 12 12
Lines 871 876 +5
==========================================
+ Hits 809 816 +7
+ Misses 34 33 -1
+ Partials 28 27 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace the custom, complex runeWidth logic with the industry-standard go-runewidth library. This improves accuracy for visual width calculations (especially for CJK characters and combining marks) and removes an unrelated maintenance burden from the project.
The go-runewidth import was grouped with the local peczenyj imports; gci (and `task ci`) wants third-party and local prefixes in separate blocks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Regression tests for the three fixes in this branch, none of which had tests: - parseNolint: multiline star-prefixed block comments (/*\n * nolint\n */) and block comments carrying a token list, which the existing test (bare /* nolint */) did not exercise. - truncPad: CJK (two-cell) runes in both the padding and truncation paths; the prior test's result was all narrow runes, so it passed even under the old rune-count implementation. - WantColor / ResolveWidth: the nil *os.File path (non-*os.File writer), which previously would have dereferenced nil. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address the codecov patch-coverage report on the changed lines: - truncPad: restructure to pad-if-fits / else truncate-with-ellipsis-and- pad. This removes the previously unreachable trailing `return "…"` (dead code the coverage tool flagged) and fixes a real bug — a wide rune straddling the truncation boundary used to leave the column one cell short, misaligning side-by-side output. No golden fixture truncates, so output for existing cases is unchanged. - add TestIsCharDeviceNil for the nil-file guard (stdoutFile passes nil for non-file writers; WantColor short-circuits so it was never exercised). - add TestStdoutFile covering both the *os.File and non-file branches. The remaining uncovered patch lines are the term.GetSize success branches in ResolveWidth, which need a real terminal (PTY) to exercise; they were uncovered on devel too and the patch only re-indented them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
This PR addresses several bugs identified during a deep review:
CI is green locally (task ci passed).