Skip to content

Add syntax highlighting for diffs#154

Merged
agavra merged 1 commit intoagavra:mainfrom
mikecfisher:feat/diff-syntax-highlighting
Feb 6, 2026
Merged

Add syntax highlighting for diffs#154
agavra merged 1 commit intoagavra:mainfrom
mikecfisher:feat/diff-syntax-highlighting

Conversation

@mikecfisher
Copy link
Copy Markdown
Contributor

@mikecfisher mikecfisher commented Feb 6, 2026

This adds proper syntax highlighting to diff views. There was already some scaffolding for it in the codebase, but it didn't actually work — the highlighter was being fed every diff line (context, additions, deletions all mixed together) in a single pass, so syntect's parser state would fall out of sync whenever additions and deletions were interleaved in a hunk. In practice that meant most real-world diffs rendered without any colors.

I rewrote the highlighting path to split the diff into two clean sequences before running syntect: one for the old file side (context + deletions) and one for the new file side (context + additions). Each side gets its own highlighting pass against coherent source, then the spans get mapped back to the original diff line order. Added and deleted lines also get their respective background colors applied on top of the syntax colors.

I also expanded the syntax detection quite a bit so more file types get highlighted:

  • Case-insensitive extension matching (e.g. MAIN.RS)
  • Shebang detection for extensionless scripts
  • Fallback mappings for extensions syntect doesn't ship with — TypeScript, Vue, Svelte, SCSS, Dockerfile, Prisma, Elixir, Nim, Kotlin, and a bunch more

All existing tests pass plus I added a bunch of new ones covering the split logic, per-line failure isolation, the fallback mappings, and the background color application.

Test plan

  • cargo test — 174 tests pass
  • Manually reviewed interleaved TypeScript hunks that were previously broken

@mikecfisher mikecfisher changed the title Fix syntax highlighting for mixed diff hunks Add syntax highlighting for diffs Feb 6, 2026
@agavra
Copy link
Copy Markdown
Owner

agavra commented Feb 6, 2026

Nice! Thanks @mikecfisher - I'll review this in a bit the approach makes a lot of sense to me much better than what's there.

Until then, let's split out the vendored-libgit2 to another change and feature guard it so that you have to install from cargo if you want to use that. IIRC, vendoring libgit2 depends on openssl which causes a bunch of problems when building binaries for various systems.

@mikecfisher mikecfisher force-pushed the feat/diff-syntax-highlighting branch 2 times, most recently from 1508151 to cde50f8 Compare February 6, 2026 23:04
The existing syntax highlighting scaffolding fed all diff lines (context,
additions, deletions interleaved) into a single syntect highlighter
pass, which broke parser state on mixed hunks so nothing actually
rendered with colors.

This splits the diff into two clean sequences (old-file and new-file),
highlights each independently, then maps results back by index. Context
lines go into both sequences so the parser sees coherent source on each
side.

Also broadens syntax detection: case-insensitive extension lookup,
shebang-based detection for extensionless files, and fallback mappings
for common extensions syntect doesn't know about (TypeScript, Vue,
Svelte, Dockerfile, etc.).
@mikecfisher mikecfisher force-pushed the feat/diff-syntax-highlighting branch from cde50f8 to cfcafee Compare February 6, 2026 23:04
@mikecfisher
Copy link
Copy Markdown
Contributor Author

Thanks @agavra . Good call makes total sense. PR updated!

Copy link
Copy Markdown
Owner

@agavra agavra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM and everything works on a local diff, but I couldn't actually reproduce the initial error condition that you describe since the diff highlighting worked for me so I can't confirm it fixes. Either way, happy to merge since the improvement makes sense logically!

@agavra agavra merged commit 73a9982 into agavra:main Feb 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants