rtfpessoa / diff2html

Pretty diff to html javascript library (diff2html)
https://diff2html.xyz
MIT License
2.79k stars 272 forks source link

Slim version doesn't fallback to 'plaintext' on unsupported file type #504

Closed scfrazer closed 10 months ago

scfrazer commented 10 months ago

Step 0: Describe your environment

Step 1: Describe the problem:

I'm an indirect user of diff2html. I use the VSCode extension vscode-diff-viewer, which uses diff2html to show diffs inside VSCode. It is using the 'slim' version. When I have a diff for a file type not supported by the slim version (Django in my case, mapped from Jinja files), an error happens and nothing is produced. I'm sorry I can't be more specific, as I said I am in indirect user and have followed error messages and layers of code around the best I can. Here is a screenshot of the error in the VSCode developer tools console:

image

I hope this is enough for you to figure out where the issue is. If not I will attempt to get you more information.

diff example:

diff --git a/tools/tracer/diagram/diagram.css.jinja b/tools/tracer/diagram/diagram.css.jinja
index 7c42bab..a2d02a5 100644
--- a/tools/tracer/diagram/diagram.css.jinja
+++ b/tools/tracer/diagram/diagram.css.jinja
@@ -87,6 +87,11 @@ g.highlighted {
   stroke-width: 4px;
 }

+g.selected {
+  stroke: {{ context.select }};
+  stroke-width: 4px;
+}
+
 rect.anchor {
   stroke: {{ context.background }};
   fill: {{ context.background }};
@@ -104,6 +109,11 @@ th {
   padding: 10px;
 }

+tr.selected {
+  stroke: {{ context.select }};
+  stroke-width: 4px;
+}
+
 td {
   padding: 10px;
 }

Observed Results:

Expected Results:

rtfpessoa commented 10 months ago

Weird, are you sure the plugin is using the latest diff2html version? Some versions ago I did some improvements in this area.

Also now you can even provide overrides for languages like this https://github.com/rtfpessoa/diff2html/blob/41a901694dc1f7a89de595555a6cd718cbf7e117/README.md?plain=1#L387

I think you need to ask for support from the plugin dev not much I can do here as it does not seem like a problem.

If you really think it is a diff2html problem, feel free to re-open but provide some extra details.

scfrazer commented 10 months ago

Thanks for the update, I'll see if I can debug the issue more, maybe use the suggestion you gave above for highlightLanguages. I'll re-open if there seems like something is generic enough to perhaps be in this code instead.

scfrazer commented 10 months ago

Hey, @rtfpessoa I think this issue should be re-opened. I just submitted a PR with a simple change to avoid causing problems in libraries that use the slim version of diff2html. I am new to the GitHub PR process so hopefully I've done everything correctly.

BTW I originally tried going down the route you suggested using highlightLanguages. I can't remember the details off the top of my head, but there was an issue that the subclass that has the highlightLanguages member wasn't exported so it was not externally usable. I'm not an expert so maybe I messed up, but FYI there may be an issue there as well.

rtfpessoa commented 10 months ago

Published v3.4.41 with the fix