Highlight is working not as supposed about diff2html HOT 19 CLOSED

Yup, that is quite bad.

I had one idea to try solving that problem.
It seems to improve overall highlight but still doesn't fix java comments.

The idea is to invoke highlight in the block and then inject the highlighted lines of the block in the HTML again.

How it is looking after the improvement:

I did one commit here: a482097

Edit: I think the main problem is that it puts the whole comment in a span and since I am splitting the comment by line it destroys the html.

I had a crazy idea for the hljs side and created an issue highlightjs/highlight.js#1167

from diff2html.

Haha, I have tried such thing (just same code as yours) yesterday, too! =). Yup, not closed span problem =). Nice try with highlightjs/highlight.js#1167, will hope that they offer something.

from diff2html.

Thanks to @isagalaev I was able to fix the highlight in affd71e .

Now highlight should be perfect except for cases where the diff
context is not enough to understand what is the code.

When highlight.js opens this part to the API I can remove the code.

from diff2html.

Wow! This is great! Thank you very much!

from diff2html.

Found new problem, now it looses "hljs" class and language class:
Was:

Now:

from diff2html.

Missing buildClassName function (from highlight API). But currently I don't where it must be. Will try to find correct place.

from diff2html.

@lantian should be fixed here 719b1cd

from diff2html.

Yes! Thanks!

from diff2html.

BTW, I also have another bug in mind. And I will try to fix it soon.

Basically we have two code snippets in each diff, the old and the new.
If we parse them together it might not make sense. I will fix this by having two states one for each part.
And this way it should be even more accurate.

from diff2html.

@lantian should be fixed here 73999bb

from diff2html.

Seems to be ok! Thank you!

from diff2html.

I will see if there are any more problems and will probably deploy this soon.

from diff2html.

👍

from diff2html.

If you guys don't mind me to weigh in, you probably don't need buldSpan() or any code from highlight.js itself to assign two classes on a container. "hljs" is a constant, and it looks like you know the language name anyway, since you call highlight() with it (and I'm not sure you even need the language in the class attribute).

All this of course is up to you, I'm just pointing out that assigning classes to an outside container is not tied to the highlighting process in any way.

from diff2html.

@lantian noticed that some colors where not being correctly set without the classes.

I am not sure if it was depending on hljs and the language or just hljs.

In my case I only know the one of the language alias, usually the extension.

@isagalaev do you think it might work only with hljs?

Edit: I checked the CSS and they seem language agnostic.
Edit2: After all I really have the language in the result. I will add it manually.

Thanks for pointing it 👍

from diff2html.

Edit: I checked the CSS and they seem language agnostic.

Yes, this was an express design goal for a big refactoring a while ago. Provided you're on version 9 or later.

do you think it might work only with hljs?

You shouldn't even need that. The only stuff that gets assigned to ".hljs" class itself is this:

.hljs {
  display: block;
  overflow-x: auto;
  padding: 0.5em;
  background: ... ; color: ... ;
}

And you have your own block formatting rules and base colors, I believe.

from diff2html.

@isagalaev actually it was @lantian not sure what were the custom theme/colors he was using.

fyi, this was released as version 2.0.0-rc.3.

from diff2html.

Cool! Always nice to see highlight.js being used in some interesting fashion :-)

from diff2html.

The theme was: "Sunburst-like style (c) Vasily Polovnyov [email protected]"
And exactly missing ".hljs" class was a problem - that class is defined in theme (with css color property) and wasn't provided by the script. From that point I have no any custom css related to highlight =), only for diff2html.

from diff2html.