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

feat: implement colour contributions for debug REPL #97762

Merged
merged 1 commit into from
May 18, 2020
Merged

feat: implement colour contributions for debug REPL #97762

merged 1 commit into from
May 18, 2020

Conversation

robertrossmann
Copy link
Contributor

@robertrossmann robertrossmann commented May 13, 2020

This PR fixes #97695.

Most of the debug REPL components are now theme-able. 🎉 🎨 The following new colour contributions have been added:

  • debugConsole.infoForeground: info-level messages
  • debugConsole.warningForeground: warning-level messages
  • debugConsole.errorForeground: error-level messages
  • debugConsole.sourceForeground: foreground colour of the source filename which generated the log message
  • debugConsoleInputIcon.foreground: icon colour which marks the evaluation input. I remember seeing a very specific name for this icon but I cannot remember. I will happily update the naming of this contribution to match if this is not accurate.

Chores

I have moved the theming support for REPL out of the repl.ts file so we have it in one place.

@isidorn
Copy link
Contributor

isidorn commented May 14, 2020

@robertrossmann great PR as usual! Thanks a lot 👏

  1. I am convinced that we need all those colors except debugRepl.expressionForeground. We already have debugTokenExpression.name and debugTokenExpression.value. And I belive that the repl should just use those colors for coloring expressions. Why should the debugRepl have different colors and not be consistent with the other debug views?
  2. We never call it debugRepl to whatever is user facing, thus I suggest that the names of the colors are debugConsole.

Adding @aeschli as a reviewer for color names
fyi @bpasero

@isidorn isidorn requested review from isidorn and aeschli May 14, 2020 09:01
@bpasero
Copy link
Member

bpasero commented May 14, 2020

Thanks for pushing this. Would be great to eventually have all of debug be fully themable 👍

@robertrossmann
Copy link
Contributor Author

@isidorn Thanks for the suggestions!

I will drop debugRepl.expressionForeground. To be honest, I don't like it that much, either. Instead, I will use the debugTokenExpression.value as the colour for the evaluated expression in the console. Though I am unsure where I would use the debugTokenExpression.name. 🤔

I'll rename the colour scope to debugConsole. Makes sense. 👍

@robertrossmann
Copy link
Contributor Author

@isidorn updated now, with some deviations noted below. ℹ️

  • I noticed that on high-contrast theme, the debugConsole.inputMarkerIconForeground was poorly visible so I cranked up the opacity back to 1 to make it stand out.
  • I tried to use the debugTokenExpression.value colour for the evaluated expressions in the REPL, but it looked... dull. The built-in themes use a dimmed-down variant of the foreground text colour for this and it made it look like a comment, or a footnote, or generally something unimportant. I ultimately decided to not allow theming this part of the UI. Another reason for this decision was that I noticed that as I type the expression into the input box, I get pretty good syntax colouring. 🎨 This colouring is currently discarded when the expression is shown in the console after I press Enter and I think it would be awesome to have this highlighting carry over to the console, making this particular colour contribution redundant. 💯

Let me know what you think. ❤️

@isidorn
Copy link
Contributor

isidorn commented May 14, 2020

Thanks a lot. Your changes look great thus approving.
As for the coloring of evaluated expressoins in the REPL, I agree with you and I think we can tackle that in a sepearte Feature Request / PR.

Though let's wait just for @aeschli to approve the color names and then we can merge this.

@isidorn isidorn added this to the May 2020 milestone May 14, 2020
@isidorn
Copy link
Contributor

isidorn commented May 15, 2020

@robertrossmann after discussing with @aeschli we like all the color names except debugConsole.inputMarkerIconForeground
Can you please change it to debugConsoleInputIcon.foreground

@robertrossmann
Copy link
Contributor Author

@isidorn Updated! ✅

@isidorn
Copy link
Contributor

isidorn commented May 18, 2020

Looks great. Thanks for doing this. Merging in 🎉

@isidorn isidorn merged commit fd9d65b into microsoft:master May 18, 2020
@robertrossmann robertrossmann deleted the feat/repl-colours branch May 18, 2020 10:13
robertrossmann added a commit to robertrossmann/vscode-remedy that referenced this pull request May 18, 2020
github-actions bot pushed a commit to robertrossmann/vscode-remedy that referenced this pull request May 18, 2020
# [4.14.0](4.13.0...4.14.0) (2020-05-18)

### Features

* add debug Console colour contributions 🎨 ([9cd02be](9cd02be)), closes [microsoft/vscode#97762](microsoft/vscode#97762)
* customise colours for Test Explorer UI 🎨 ([0d0568f](0d0568f)), closes [hbenl/vscode-test-explorer#132](hbenl/vscode-test-explorer#132)
@github-actions github-actions bot locked and limited conversation to collaborators Jul 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make repl themable
3 participants