Report real line numbers after an --ignore-multiline-regex region#3981
Open
chuenchen309 wants to merge 1 commit into
Open
Report real line numbers after an --ignore-multiline-regex region#3981chuenchen309 wants to merge 1 commit into
chuenchen309 wants to merge 1 commit into
Conversation
get_lines derived each fragment's line number by accumulating len(splitlines()) and subtracting one for the ignored match. That only holds when the match starts at column 0 and does not end on a line boundary, so misspellings after an ignored region were reported one line off in either direction -- including with the example from --help. Take each fragment's line number from its own offset instead. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Misspellings that appear after an
--ignore-multiline-regexregion are reported with the wrong line number — including with the exact example given in--help.Reproducer
ml.txt:The typo is really on line 6, and plain
codespell ml.txtagrees (it reports lines 3 and 6). With the option, the surviving typo moves:codespell ml.txtml.txt:3,ml.txt:6--ignore-multiline-regex '# codespell:ignore-begin *\n.*# codespell:ignore-end *\n'— the example from--helpml.txt:5ml.txt:6--ignore-multiline-regex 'codespell:ignore-begin.*codespell:ignore-end'— the form the test suite usesml.txt:7ml.txt:6Both directions of error are reachable. The region itself is ignored correctly in both cases, and
--write-changesstill edits the right bytes; only the reported line number (and theFIXED:output) is off.Cause
get_linestracks the line number by accumulatinglen(lines)per fragment and then subtracting one for the ignored match:Splitting a slice tells you how many lines it touches, not how many line breaks it crosses. The
- 1is a correction for the case where the match begins at column 0 and does not end on a line boundary — the leading partial line and the trailing partial line are each counted once too often, and the two errors cancel. Change either property and the cancellation breaks:--helpexample ends with\n, so there is no trailing partial line and the- 1over-subtracts → one line too low;codespell:ignore-begin.*starts mid-line (after#), so the preceding fragment's"# "remainder is counted as a whole line → one line too high.Fix
Take each fragment's line number from its own start offset, which is the definition of the thing being computed:
No fragment content changes — only the line number attached to it — so detection and
--write-changesbehaviour are untouched.Tests
test_ignore_multiline_regex_line_numbersasserts the reported line number for both regex forms above. There was no existing coverage for this: the current--ignore-multiline-regextests assert error counts and post-fix file contents, never line numbers, which is why the bug survived.Full suite is unchanged otherwise: 124 passed vs. 123 on
main(the one extra is this new test), with the same pre-existing failures on my sandbox in both runs.ruff check/ruff formatclean; both of CI's codespell self-checks still behave as expected.Disclosure
AI-assisted, and I'd rather say so plainly. The reproducer above, the
main-vs-branch baseline comparison, and the--help-example finding are all things I ran and read myself; the root-cause reasoning about the two cancelling off-by-ones is likewise checked against the actual behaviour rather than asserted. Happy to adjust scope or naming if you'd prefer something narrower.