Conversation
Owner
|
These are two separate things, can you split them? I agree that the highlighting queries should win over extras for deciding string vs comment. The second case for handling empty regions feels like it might need a fix elsewhere. |
Wilfred
added a commit
that referenced
this pull request
Sep 2, 2026
Ruby uses extra nodes for heredocs, but they're strings from difftastic's viewpoint. See discussion in #1052.
MatchedPos::new trims zero-width spans off the ends of an atom's position list so no MatchedPos is emitted for an empty edge line. It then handed the trimmed list to split_atom_words, which treats pos[0] as the origin for byte offsets into the atom's content. When the leading span was dropped, offset 0 no longer pointed at the atom's first line and every word was reported one line too far down. Ruby heredoc bodies hit this every time, because a body starts with a newline and so always has an empty first span: the whole heredoc was marked changed, highlight runs ended at the previous line's length, and the following `end` was overdrawn. The same stale span indexed past the display line vector, panicking --display side-by-side-show-both. Ordinary strings escaped it because their first line contains the opening quote.
Author
|
I've removed the similar 843160a introduced commit. |
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.
Why
Fixes #994.
MatchedPos::newtrims zero-width spans from the ends of an atom's position list, then passes the trimmed list tosplit_atom_words, which usespos[0]as the origin for byte offsets into the atom's content. Dropping the leading span leaves offset 0 pointing at the wrong line, so every word is reported one line too far down. A Rubyheredoc_bodyalways has an empty first span, because the body starts with a newline — so heredocs hit this every time: the whole heredoc is marked changed, highlight runs end at the previous line's length, and theendafter the heredoc is overdrawn. The same stale span indexes past the display line vector, panicking--display side-by-side-show-both. Ordinary string literals escape it because their first line holds the opening quote. tree-sitter-ruby parses these heredocs correctly; the bug is on difftastic's side.How
split_atom_wordsnow receives the untrimmed spans, so its offset arithmetic starts from the atom's real first line, and trims internally on the path that reports whole lines rather than words. A second commit fixes a related misclassification: difftastic infers "comment" from tree-sitter'sis_extra(), but Ruby listsheredoc_bodyinextrasbecause heredoc bodies occur out of source order, so heredocs were styled as comments and discarded by--ignore-comments. That heuristic now yields to the parser's highlight query when the query marks the node as a string. Includes a regression test for an atom with a zero-width leading span.Written with AI assistance (Claude Code).