1159730 - Javascript template string (with backquotes/ticks) aren't syntax highlighted correctly in mozreview diffs

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P2)

Description

[Patrick Brosset <:pbro>]

Attachment: syntax-highlight-problems.png

If a file diff on mozreview contains a javascript template string:

let str = `this is a template string with an ${expression}`;

then the syntax highlighting of that string is wrong, and the rest of the file is incorrectly highlighted too.

`...` strings should be at least treated like "..." strings. Bonus points if the ${...} get highlighted too.

Comment 1

[Gregory Szorc [:gps]]

Syntax highlighting is done via Pygments. File a bug against Pygments to support ES6 syntax and it will eventually get deployed.

Comment 2

[Patrick Brosset <:pbro>]

Found the issue on Pygments' repo: https://bitbucket.org/birkenfeld/pygments-main/issue/1100/add-support-for-ecmascript-6

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Although it's a bug from the upstream project, it is a bug which should be fixed. I think that means it shouldn't be marked as WONTFIX. We can probably just leave it open until the upstream fixes it and we start using the fixed version.

(BTW, I cannot add the url in comment 2 to See Also field. That looks like a bug we should fix in Bugzilla...)

Comment 4

[Nathan Froyd [:froydnj]]

Ran into this today when backticks were surrounded with red and mismatched quotes inside template strings caused mis-highlighting of the entire file.

Looks like the pygments issue has been fixed:

https://bitbucket.org/birkenfeld/pygments-main/commits/3ecc5b1c8c30

How complicated is this to deploy now?

Comment 5

[Gregory Szorc [:gps]]

We deploy a custom Pygments distribution from https://hg.mozilla.org/users/gszorc_mozilla.com/pygments/ because Pygments historically Pygments releases are seldom.

I see they *finally* released a new major version recently. So we can likely push that to my user repo and update the provisioning code to reference the new SHA-1. We may want to spot check things on the dev server to verify the new version didn't break any APIs. I have no clue what Pygments' API compat story is...

Comment 6

[Gregory Szorc [:gps]]

Attachment: MozReview Request: ansible: upgrade to Pygments 2.1.1 (bug 1159730); r?smacleod

This will upgrade Pygments on Mercurial and MozReview to 2.1.1.
Previously we were running 2.0.1 with a cherry-picked patch to
support .jsm detection as JavaScript.

A quick glance at the diff between the packages doesn't seem to reveal
any significant API changes. So this will hopefully be a drop-in
replacement.

Review commit: https://reviewboard.mozilla.org/r/35699/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35699/

Comment 7

[Steven MacLeod [:smacleod]]

Comment on attachment 8721541 [details]
MozReview Request: ansible: upgrade to Pygments 2.1.1 (bug 1159730); r?smacleod

https://reviewboard.mozilla.org/r/35699/#review32491

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/6d7fec6a6fac

I'll possibly deploy this Tuesday.

Comment 9

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/0d4ebd4464bd7889ea8c28670fc2c1e9f76c8aee
ansible: install Pygments through pip (bug 1159730)