Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offer theme colors for text foreground in diff editor #71663

Closed
octref opened this issue Apr 3, 2019 · 7 comments
Closed

Offer theme colors for text foreground in diff editor #71663

octref opened this issue Apr 3, 2019 · 7 comments
Assignees
Labels
themes Color theme issues

Comments

@octref
Copy link
Contributor

octref commented Apr 3, 2019

Abyss:

image

Monokai:

image

The low color contrast makes the text hardly readable. Also an issue for Nord.

nordtheme/visual-studio-code#14 (comment)

It would be great if theme authors could say, in the diff added/removed region, use a different foreground than the normal foreground to avoid contrast issues.

@aeschli
Copy link
Contributor

aeschli commented Apr 4, 2019

But then you would lose syntax highlighting for the diff regions.

IMO it's up the themes to find good diff background colors that go well with the token colors. It's the same for all other background colors in the editor (selection, ...)

@aeschli
Copy link
Contributor

aeschli commented Apr 4, 2019

In order to replace all tokens with a foreground color the diff view has to create an annotation with that color. It would only do that when the color is defined.

I'd rather not go there, but expect themes to pick good colors (it's all under their control)

@aeschli aeschli closed this as completed Apr 4, 2019
@octref
Copy link
Contributor Author

octref commented Apr 4, 2019

I meant being able to say, for added/removed regions, use different colors for certain syntaxes. For example,

    {
      "name": "Comment",
      "scope": "comment",
      "settings": {
        "foreground": "#616E88",
        "git.added.foreground": "<Color good for the green>",
        "git.removed.foreground": "<Color good for the red>",
      }

@octref
Copy link
Contributor Author

octref commented Apr 8, 2019

@aeschli Can we reopen this as a feature request?

@aeschli
Copy link
Contributor

aeschli commented Apr 8, 2019

We can open it to collect votes, but given the complexity of the implementation (a layering issue, token colors are computed in vscode-textmate, the git decoration comes from an extension), I don't see a way this can become reality.
I'd rather have theme pick good colors.

@octref
Copy link
Contributor Author

octref commented Apr 8, 2019

@aeschli @alexandrudima Is it possible to do this?

  • Currently we have rules for .mtk<n> classes for different syntax tokens
  • When the code is in diff region, add .git-added or .git-removed to the container
  • The .git-added .mtk<n> and .git-removed .mtk<n> CSS rules would apply the colors

@arcticicestudio
Copy link

arcticicestudio commented Apr 11, 2019

@aeschli I'm not familiar with the parsing/coloring architecture of VS Code so I can't make any statements how to solve this or how this would impact the performance. I know that I've spend many hours trying to find a color mix that keeps the balance between the theme ambience and the readability and it looks like it is almost impossible. There are many other themes with the same problem and all authors are helpless and limited regarding the design possibilities, e.g. see the discussion in nordtheme/visual-studio-code#105 (there are also more closed issues).
Choosing a good color for the diff means on the other side it will completely change (or even destroy) the ambience of the theme for all other views, e.g. using a darker and more saturated color for comment would help in the case of the Nord theme to make comment readable in diff-addition views, but would make them almost invisible in other views (dark foreground on dark background).
Also, like @octref also showed in the entry post, not only custom/third-party themes are affected but the bundled ones where the comment for the Abyss and Monokai themes are almost not visible at all.

It would be really nice if this would be evaluated again to see if e.g. the solution from @octref can be used to solve this without impact on the performance. The changes only need to apply to the view/tab that gets opened when using the Git diff feature.

If this is also to much effort/performance impact I'd suggest to check if there is a way to invert all foreground colors when they are within the Git diff addition/remove scope. This would reflect the design of other editors like Vim (see Nord Vim diff background feature as example). Of course this should be made a opt-in with a setting key to allow users to only enable it when it fits their needs.

@vscodebot vscodebot bot locked and limited conversation to collaborators May 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
themes Color theme issues
Projects
None yet
Development

No branches or pull requests

3 participants