New issue
Advanced search Search tips
Starred by 4 users

Issue metadata

Status: Released
Owner: ----
Closed: Jul 2018
Cc:
Components:
ReleasedIn: 526.0



Sign in to add a comment
link

Issue 9389: Enable HighlightJS Clojure support for PolyGerrit

Reported by m...@talios.com, Jul 4 2018

Issue description

Affected Version:

What steps will reproduce the problem?
1. Switch to the PolyGerrit GUI
2. Look at a .clj, .cljs, or .edn file in a Gerrit review
3. 

* What is the expected output?

I'd expect to see clojure syntax highlighting as shown on the HightlightJS website, with symbols, labels, parens, function calls etc. highlighted.

* What do you see instead?

The code isn't highlighted in any specific Clojure manner, only quoted strings
 

Comment 1 by m...@talios.com, Jul 4 2018

This is against 2.15.2, would be nice to see this included in 2.15.3 if possible.

Comment 2 by david.os...@gmail.com, Jul 8 2018

Project Member
I can confirm the problem.

I tried to debug it, and to simplify the debug session,
I've built highlight.js without minification (just copied
non-minified version over lib/highlightjs/highlight.min.js
file).

While debugging, I can see, that the language was correctly
discovered as clojure (extension was .clj) and that the
process method correctly processing the test Clojure code,
line by line. At the end the annotated code is shown, but
nothing happened.

For example the first line:

"(def ^:dynamic chunk-size 17)"

is converted to:

<div class="style-scope gr-diff contentText" data-side="right">(<hl class="gr-diff gr-syntax gr-syntax-name">def</hl> ^:dynamic chunk-size <hl class="gr-diff gr-syntax gr-syntax-number">17</hl>)
</div>

Here is the screenshot: [1].

* [1] https://imgur.com/a/cpnOMnL

Comment 3 by david.os...@gmail.com, Jul 8 2018

Project Member
Cc: thomasmu...@yahoo.com

Comment 4 by david.os...@gmail.com, Jul 8 2018

Project Member
Status: Accepted (was: New)
OK, comparing this to working Java source highlighting, I see in fact this difference,
on this line:

"class Demo extends Sum {"

hljs producing:

<div class="style-scope gr-diff contentText" data-side="right">
<hl class="gr-diff gr-syntax gr-syntax-keyword">class</hl> <hl class="gr-diff gr-syntax gr-syntax-title">Demo</hl>
<hl class="gr-diff gr-syntax gr-syntax-keyword">extends</hl> <hl class="gr-diff gr-syntax gr-syntax-title">Sum</hl>{</div>

So clearly, highlight.js doing something different on Clojure code.

I thing the reason is how the language defined in HL.js. While
in Java mode the keywords are defined as keywords [1], in Clojure
mode they are defined as builtin-name: [2].

The styling difference is: gr-syntax-name vs. gr-syntax-keyword.

It appears that there is no style definition for gr-syntax-name,
even though this is accepted in whitelist: [3].

Not sure, if it the correct way to fix it: style gr-syntax-name exactly
the same way as gr-syntax-keyword fixed syntax highlighting for Clojure.

Here the screenshot in dark theme: [4].

Hint: to activate dark theme, just open the JS console in your browser
and run: localStorage.setItem("dark-theme", "1");

[1] https://github.com/isagalaev/highlight.js/blob/master/src/languages/java.js#L40
[2] https://github.com/isagalaev/highlight.js/blob/master/src/languages/clojure.js#L11
[3] https://github.com/GerritCodeReview/gerrit/blob/master/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js#L69
[4] https://imgur.com/a/QPyu2cD

Comment 5 by thomasmu...@yahoo.com, Jul 8 2018

Project Member
It may be whitelisted but did we add it to the styles with a colour?

Comment 7 by david.os...@gmail.com, Jul 8 2018

Project Member
Status: ChangeUnderReview (was: Accepted)
https://gerrit-review.googlesource.com/c/gerrit/+/187893

Comment 9 by david.os...@gmail.com, Jul 10 2018

Project Member
Status: Submitted (was: ChangeUnderReview)

Comment 10 by david.pu...@gmail.com, Jul 18 2018

Labels: FixedIn-2.15.3

Comment 11 by david.pu...@gmail.com, Jul 18 2018

Status: Released (was: Submitted)

Comment 12 by wyatta@google.com, Jul 24 2018

Project Member
ReleasedIn: 526.0

Sign in to add a comment