Syntax highlighting in Gerrit

[Alexei Svitkine]

Hi chromium-dev,

Now that there's a good number of reviews coming in via Gerrit, I've noticed that I'm not a fan of its syntax highlighting.

It appears to use a different highlighting scheme than Rietveld. Now, it doesn't mean that Rietveld's scheme is the best one - but I tried to look around and Gerrit's scheme also seems to different from what we see in codesearch and also different from the scheme used in the internal Google code review system.

So I could see an argument if the new scheme was more consistent with other tools, but in this case it seems to actually be instead diverging from the other tools. (The most obvious is the difference in the colors for keywords going from blue to pink, but there's a number of other differences such as ifdefs being mysteriously colored bright red in Gerrit.)

So I'm curious, was this something that has been previously discussed somewhere and concluded that we want to go this way - or has this just not been brought up before? If it's the latter, can it be changed? At the very least I think we should aim for consistency between our code review and code search tools.

-Alexei

[Patrick]

+1 I would love a way to select between different highlighting choices.

[Aaron Gable]

The best way to communicate these concerns is to file a bug at crbug.com/gerrit/new using the PolyGerrit template.

There has not been extensive discussion of the syntax highlighting color scheme on this list or anywhere else -- it is currently just the default scheme used by the syntax highlighting library, I believe.

We probably won't be providing multiple color schemes ton choose from before launch, simply because Rietveld didn't either. But we hope to do so in the future, and we can certainly talk about changes to the default scheme if something needs to be changed.

Anyway, please file a bug and we can continue discussion there.

Thanks,
Aaron

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/be0a41c7-d4d2-4412-922a-0c078a5f9ace%40chromium.org.

[Alexei Svitkine]

Thanks, filed: https://bugs.chromium.org/p/gerrit/issues/detail?id=6416

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2ciXxwWRFT4AEe5FBW-EMO9wUPB_-WQw_Jj6CNF05ZQ_A%40mail.gmail.com.

[Marc-Antoine Ruel]

Aaron, I had filed https://bugs.chromium.org/p/gerrit/issues/detail?id=5528 months ago. :) I'm not asking for options, which I think is wrong, we don't need more options, I think just using more reasonable colors would "fix" the issue.

IMHO, Eclipse is a prime example of anti-UX designed by committee.

</free_unasked_for_opinion>

M-A

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKFU2SDOfu9W5t00qJmGZwy80Jq1viXdLJoU1ekQVjtVcmd54g%40mail.gmail.com.

[Aaron Gable]

Thanks for pointing to that issue. It's asking for a different thing -- changing the background color in diff chunks, as opposed to changing the text color of identifiers and builtins -- so I'm not going to dedupe them into each other, but they're both worth considering.