gex icon indicating copy to clipboard operation
gex copied to clipboard

Syntect highlighting experiment

Open CptPotato opened this issue 2 years ago • 4 comments

Adds optional syntax highlighting based on syntect to hunk diffs.

screenshots

image

with background color: image


Remaining things to fix/consider (not all changes are pushed to the branch, yet):

  • [x] ~~Use separate highlighter instance for added and removed lines to keep the syntax intact.~~
    • Highlighting hunks in isolation is incorrect.
    • [ ] Highlight the whole file first to ensure syntax in each hunk is correct.
  • [x] ~~Add inline highlighting for single line changes.~~
    • Works but breaks easily and I'm not happy with the code. I probably won't include it in this PR.
  • [ ] Don't feed the hunk marker @@...@@ into the highlighter.
  • [ ] Increase hunk header visibility. Right now it blends in with the highlighted code too much.
  • [x] Try highlighting only the selected hunk for visual aid. Display others in gray.
    • [ ] Highlight all hunks when the cursor is on the file.
    • [ ] Make this optional.
  • [ ] Highlight untracked files.
  • [x] Check for interference with the existing highlighting.
    • [ ] Highlight trailing whitespaces.
  • [ ] Add configuration options.
  • [x] ~~Try highlighting the background to the whole width of the terminal?~~
    • Probably "out of scope" for this PR.
  • [x] ~~Use syntax theme background color?~~
    • Requires highlighting to the full width of the terminal to look good, Probably "out of scope" for this PR.

CptPotato avatar Sep 22 '23 11:09 CptPotato

Looking amazing so far, thanks for opening the PR!

Can't wait for this to be ready for review, if you stick with it.

Piturnah avatar Sep 22 '23 17:09 Piturnah

The code is starting to look a little better, though there's still a few things to do.


I made the syntax highlighting optional (still need to fix the config, though). When disabled it uses the current look.

Also, I tried to see how it looks when only the selected hunk is highlighted:

screenshot

image

I personally like it, what do you think? I could also see if I can make this optional.

CptPotato avatar Sep 24 '23 14:09 CptPotato

Thanks for the first review, appreciate it! I'll probably take my time with this PR.

One problem I didn't notice until now is that highlighting each hunk in isolation is wrong, since it doesn't always produce valid syntax. So I'll have to perform highlighting to the whole file first and then take the highlighted lines for each hunk from there. I'll probably cut some corners for now and only highlight the "new" part of the diff, which should be fairly easy. Looking at delta's output it actually seems to do the same thing. To highlight the deleted lines I'd have to reproduce the old version of the file first, which can be implemented at a later point if it's needed.

CptPotato avatar Oct 02 '23 17:10 CptPotato

Thanks for the first review, appreciate it! I'll probably take my time with this PR.

Absolutely take your time. Thanks again for taking a look at this!

One problem I didn't notice until now is that highlighting each hunk in isolation is wrong, since it doesn't always produce valid syntax. So I'll have to perform highlighting to the whole file first and then take the highlighted lines for each hunk from there. I'll probably cut some corners for now and only highlight the "new" part of the diff, which should be fairly easy. Looking at delta's output it actually seems to do the same thing. To highlight the deleted lines I'd have to reproduce the old version of the file first, which can be implemented at a later point if it's needed.

Seems good to me!

Piturnah avatar Oct 02 '23 17:10 Piturnah