dandavison / delta

A syntax-highlighting pager for git, diff, grep, and blame output
https://dandavison.github.io/delta/
MIT License
21.43k stars 361 forks source link

🐛 Vue SFC syntax highlighting does not work if opening tag is not included in diff #1546

Open pm0u opened 9 months ago

pm0u commented 9 months ago

Edit: see https://github.com/dandavison/delta/issues/1546#issuecomment-1739844657

When diffing Vue SFC files, delta does not properly syntax highlight - even when the diff is a syntax complete file.

diff --git ***/***/test.vue
index e69de29bb..ef35203ac 100644
--- a/***/***/test.vue
+++ b/***/***/test.vue
@@ -0,0 +1,15 @@
+<template>
+  <div class="className" />
+</template>
+
+<script setup>
+import fs from 'node:fs'
+
+fs.readFile()
+</script>
+
+<style>
+.css {
+  background: transparent;
+}
+</style>
image

bat for the same file

image
pm0u commented 9 months ago

Edit: take it back, it appears to syntax highlight correctly outside of the diff contents (that's probably my bad) however CSS does not seem to highlight correctly.

image

Highlighted in bat:

image
pm0u commented 9 months ago

Closing - this was because in my delta config I had overrode the foreground colors for plus/plus-emph - this needed to be syntax - woo!

Great tool!

pm0u commented 9 months ago

Reopening as I am in fact having some issues still with Vue SFC - both in JS and CSS.. will update as I can, just tracking for now...

pm0u commented 9 months ago

Ok I believe I've isolated this, and I think I have seen similar issues -

If the diff does not include the <script> tag then the JS is not correctly highlighted.

Working (contents from blank file)

image
@@ -0,0 +1,15 @@
+<template>
+  <div class="className" />
+</template>
+
+<script setup>
+import fs from 'node:fs'
+
+fs.readFile()
+</script>
+
+<style>
+.css {
+  background: transparent;
+}
+</style>

Not working - added a console.log() far away from the script tag opening

image
@@ -15,6 +15,8 @@ console.log('')
 console.log('')
 console.log('')
 console.log('')
+console.log('')
+
 console.log('')
 </script>

Full file in bat:

image
pm0u commented 9 months ago

related: https://github.com/dandavison/delta/issues/162

Unsure if a similar workaround is available for Vue SFC

pm0u commented 9 months ago

I do not believe a similar fix would work, as the issue is that the opening and closing tags are needed to understand what part of the file we are in (I think).

Curiously when I force bat to only output lines that exclude the opening it still works - but perhaps it still parses the full file to infer syntax.

image image
pm0u commented 9 months ago

If I set the context to something huge like 200 then it highlights correctly as it manages to include the opening tag for script, style, etc. However this makes the diffs not very useful. It would be nice to have something like what bat must be doing where it parses the whole file and then displays the small section of it, although I realize this is relying on diff output

dandavison commented 5 months ago

Hi @pm0u, I think this is a dup of https://github.com/dandavison/delta/issues/117, right?

There is an easy-sounding fix that a couple of people have suggested and which someone should implement: add a -U option to delta, so that large context can be passed in, the syntax highlighting will probably be correct, and then delta can narrow the context down.

pm0u commented 5 months ago

I think that would do it, yes. Although providing an arbitrary number could be challenging (or I would likely just pass a giant number like 99999) since the boundaries of different parts of the file would be hard to predict. In this case, something like a full file option could be more succint but either would work.

dandavison commented 5 months ago

something like a full file option could be more succint

Right, but, this is out of delta's control. It is git that is generating the diff, and so it is git that controls how much context is supplied to delta. AFAIK git just offers

       -U<n>, --unified=<n>
           Generate diffs with <n> lines of context instead of the usual three. Implies --patch.

so yes, passing a "large" integer is how we'd use it. E.g.

git diff -U99999 | delta -U10
pm0u commented 5 months ago

Ah gotcha, it wasn't clear to me who was interpreting the flag in question. So then I assume the -U for delta then limits back down the context? I have confirmed in the past that a huge context (or one that spans the whole file) fixes the issue for me - but also made the diff quite useless as I'm sure you've found. The addition of a context for delta would fix this for me