Conversation
|
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 |
1508151 to
cde50f8
Compare
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.).
cde50f8 to
cfcafee
Compare
|
Thanks @agavra . Good call makes total sense. PR updated! |
agavra
left a comment
There was a problem hiding this comment.
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!
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:
MAIN.RS)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