Skip to content

fix(highlight): account for carriage return at EOF and chunk ends#4375

Merged
amaanq merged 2 commits intotree-sitter:masterfrom
thaliaarchi:cr-at-eof
Jun 5, 2025
Merged

fix(highlight): account for carriage return at EOF and chunk ends#4375
amaanq merged 2 commits intotree-sitter:masterfrom
thaliaarchi:cr-at-eof

Conversation

@thaliaarchi
Copy link
Contributor

@thaliaarchi thaliaarchi commented Apr 19, 2025

Highlighting a carriage return is only attempted when it is not the last character in a Source chunk. This skips highlighting for CR in two cases: CR as the last character in a file and CR just before a highlight event.

For the first, although EOF can be considered a line ending, in a sense, CR+EOF shouldn't count as a CRLF line ending, so such a CR should be highlighted.

For the second, it is more complicated. Since text is highlighted as a stream of events (HighlightStart, HighlightEnd, and Source), we cannot know while in a Source event whether we are the last chunk without peeking forward through HighlightStart and -End events to the next Source. Store the offset in the rendered HTML of the last CR so we can render it once we get to the next Source, if it's not part of a CRLF.

The updated test tests both of these cases. Commit 422e74f (chore: update javascript-relevant tests, 2024-02-02) added the FIXME after updating the JavaScript grammar, which caused b to become parsed as a variable, triggering the second case.

@clason clason added the highlight Ralated to `tree-sitter-highlight` functionality label Apr 19, 2025
Highlighting a carriage return is only attempted when it is not the last
character in a `Source` chunk. This skips highlighting for CR in two
cases: CR as the last character in a file and CR just before a highlight
event.

For the first, although EOF can be considered a line ending, in a sense,
CR+EOF shouldn't count as a CRLF line ending, so such a CR should be
highlighted.

For the second, it is more complicated. Since text is highlighted as a
stream of events (`HighlightStart`, `HighlightEnd`, and `Source`), we
cannot know while in a `Source` event whether we are the last chunk
without peeking forward through `HighlightStart` and -`End` events to
the next `Source`. The API takes an `Iterator`, so this isn't possible.
Luckily, it probably doesn't matter in practice, because grammars are
highly unlikely to want to split CR and LF with highlighting, so the
next `Source` won't start with LF anyways.

The updated test tests both of these cases. Commit 422e74f (chore:
update javascript-relevant tests, 2024-02-02) added the FIXME after
updating the JavaScript grammar, which caused `b` to become parsed as a
variable, triggering the second case.
@thaliaarchi thaliaarchi changed the title Highlight carriage return at EOF Highlight carriage return at EOF and chunk ends Apr 19, 2025
If a CRLF is split between two `HighlightEvent`s, the previous commit
would highlight the CR, although it shouldn't. Fix that by tracking the
offset in the rendered HTML of the last CR and insert there once known.
@thaliaarchi
Copy link
Contributor Author

The second commit builds upon the first to fix CR and LF straddling two Source regions with HighlightStart or HighlightEnd between.

@thaliaarchi
Copy link
Contributor Author

The failed test is due to a timeout:

The job running on runner GitHub Actions 7 has exceeded the maximum execution time of 20 minutes.

It seems spurious.

noorosa added a commit to noorosa/tree-sitter that referenced this pull request May 8, 2025
@noorosa noorosa mentioned this pull request May 8, 2025
@amaanq amaanq changed the title Highlight carriage return at EOF and chunk ends fix(highlight): account for carriage return at EOF and chunk ends Jun 5, 2025
@amaanq
Copy link
Member

amaanq commented Jun 5, 2025

Nice, thank you!

@amaanq amaanq merged commit 6ba73fd into tree-sitter:master Jun 5, 2025
18 checks passed
@amaanq amaanq added the ci:backport release-0.25 Backport label label Jun 5, 2025
@tree-sitter-ci-bot
Copy link

Successfully created backport PR for release-0.25:

@tree-sitter-ci-bot
Copy link

Git push to origin failed for release-0.25 with exitcode 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:backport release-0.25 Backport label highlight Ralated to `tree-sitter-highlight` functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants