Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
pm0u opened this issue Sep 28, 2023 · 11 comments
Open
1 task done

Comments

@pm0u
Copy link

pm0u commented Sep 28, 2023

Edit: see #1546 (comment)

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>
  • A screenshot of Delta's output is often helpful also.
image

bat for the same file
image

@pm0u
Copy link
Author

pm0u commented Sep 28, 2023

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
Copy link
Author

pm0u commented Sep 28, 2023

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 pm0u closed this as completed Sep 28, 2023
@pm0u pm0u reopened this Sep 28, 2023
@pm0u
Copy link
Author

pm0u commented Sep 28, 2023

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
Copy link
Author

pm0u commented Sep 28, 2023

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 pm0u changed the title 🐛 Vue SFC syntax highlighting does not work, differs from bat output 🐛 Vue SFC syntax highlighting does not work if opening tag is not included in diff Sep 28, 2023
@pm0u
Copy link
Author

pm0u commented Sep 28, 2023

related: #162

Unsure if a similar workaround is available for Vue SFC

@pm0u
Copy link
Author

pm0u commented Sep 28, 2023

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
Copy link
Author

pm0u commented Sep 29, 2023

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
Copy link
Owner

Hi @pm0u, I think this is a dup of #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
Copy link
Author

pm0u commented Jan 22, 2024

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
Copy link
Owner

dandavison commented Jan 22, 2024

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
Copy link
Author

pm0u commented Jan 22, 2024

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

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

No branches or pull requests

2 participants