sourcegraph / sourcegraph

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
9.97k stars 1.21k forks source link

Syntax highlighting #20642

Closed rrhyne closed 5 months ago

rrhyne commented 3 years ago

Change our current syntax highlightinhg color scheme to AtomOne and AtomOneDark.

felixfbecker commented 3 years ago

@rrhyne is there anything to read on how the design team arrived at AtomOne being the one we should switch to? I realize different devs all have different choices and it will never be possible to make everyone happy with any choice, but at the same time defaults are important, and there are some objective arguments you can make for or against a theme.

If I try to think fully objectively here and put my own personal preferences aside:

If I try to judge this syntax highlighting theme objectively, it really does not seem like a good theme to me. I'm sure there are people that like it and would pick it when we offer customization but I don't know if it's a good choice for a default (especially while we don't have customization).

rrhyne commented 3 years ago

@felixfbecker, I chose the theme primarily based on a few criteria:

Overall, I disagree with one of your main positions. I don't think we can design for familiar patterns based on a few token styles aligning between default IDE themes and services (github/gitlab, etc). If I spend 6 hours of my day in an IDE with a custom theme, when I switch to Sourcegraph, I'm going to be confused if the two do not match 100%. I also don't think it's possible to objectively define what is actually familiar to a very wide and opinionated user population who all use different editors and code hosts. As an exercise, we could try to find commonalities between the themes in the workspaces of Sourcegraph blog post, but at a glance feel this is a losing proposition.

Therefore, I think the ONLY way to solve this problem a is to align to a different portion of that design principle: "embracing personalization and adaptability". This means we should quickly launch the ability to customize your theme based on the most common format available to a wide range of IDEs.

All that said, I appreciate your arguments especially your detailed analysis of what level of language features the theme provides differentiation for. I'm not sure if I'm looking at this correctly (my code examples surely do not align with yours), but I didn't come to the same findings as you did. Could you have a look at my example file here and possibly improve it, and comment on the screengrabs I took in VS Code, VS Code atom one, Atom and GitHub?

Perhaps we could use this (or your improved version) as a base to look at reviewing our default theme selection?

pdubroy commented 3 years ago

My 2 cents:

I do think this is mostly a design decision and it's not really possible to make an "objectively correct" decision here. Overall I agree with @rrhyne that "inconsistency is the only consistency" — there is a huge variety of themes, and any single one we pick will only match the theme of a small fraction of users.

That said, I do agree with @felixfbecker that when I look at code under Atom One, there's a lot of red and it really sticks out visually. We can't be consistent, but I do still think there's an argument for picking something that's not so loud.

Regarding the other points:

E.g. strings are green, which is neither the case for VS Code (brown) or GitHub (grayish blue), and traditionally green is often associated with comments (if those aren't gray).

I don't know this is really true, I think green is also common for strings. E.g., Chromium code search uses green, also about half the default themes in VSCode use green or yellow-green for strings.

Green and red being heavily used for the most common elements can cause contrast issues in diff views, where the background is also a shade of green or red.

This is a good point, but could be addressed by choosing the right colors in the diff view. For example, here's what diffs look like in VSCode with Atom One:

CleanShot 2021-05-21 at 10 40 01

Variables (in declarations) and types/class/interface names are the same color (orange), which means the theme gives no visual cues to separate the "value realm" and the "types realm" (this is done by the VS Code theme for example). There is no color distinction/variation between control flow keywords (if, else, while, for, await, try, catch, throw) and declarative keywords (function, class, async, private, public, abstract).

I have personally never thought that deeply about the semantics conveyed by the color scheme. I'd be surprised if anything more than a tiny percentage of developers would notice these points. But maybe I'm wrong 🤷‍♀️

rrhyne commented 3 years ago

I've updated the Figma explorations and there are no issues with red or green text over our diff add/remove colors:

https://www.figma.com/file/XVUBnYSgJ0UzAz3FOjAaaf/syntax-highlighting-study?node-id=0%3A1&viewport=1032%2C381%2C0.26370757818222046

rrhyne commented 3 years ago

This ticket can be broken up into stages:

camdencheek commented 5 months ago

Closing as stale