Codicon color and URI support to TerminalOptions #13413
Conversation
| const iconClass = getIconUris(options.iconPath); | ||
| if (iconClass) { | ||
| return MonacoThemeIcon.asClassName(iconClass); | ||
| export function getIconClass( |
There was a problem hiding this comment.
The definition of the icon in the TerminalOptions is basically the same as in the QuickInputItem. Can't we reuse the same logic for transferring and and resolving icons here? It would make sense (IMO) to solve this the same way everywhere it applies.
There was a problem hiding this comment.
@tsmaeder
I think it is a good idea to have a utility that we can re-use. Is it something we can address in a separate PR?
Regarding QuickInputItem, I can't find any related code around this name, or do you mean QuickPickItem?
There was a problem hiding this comment.
QuickPickItem is what I meant. See quick-open.ts, etc.
There was a problem hiding this comment.
Doing stuff in a separate PR is fine with me, as long as we have a clear timeline on when it's going to happen. I wouldn't just want to "throw it on the pile".
There was a problem hiding this comment.
We now support the quick pick iconPath property.
| @@ -306,6 +316,28 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget | |||
| this.term.options.fastScrollSensitivity = this.preferences.get('terminal.integrated.fastScrollSensitivity'); | |||
| } | |||
|
|
|||
| protected isTerminalAnsiColor(color: string): boolean { | |||
There was a problem hiding this comment.
If I understand correctly, the icon is not in the terminal, but in the widget title. Why would we go through the terminal theme service here?
There was a problem hiding this comment.
@tsmaeder
I'm not quite sure what you mean. But I think in this function the author checks if the configured color is one of the predefined colors from packages/terminal/src/browser/terminal-theme-service.ts that are preconfigured in terminalAnsiColorMap.
What would you suggest to change here?
There was a problem hiding this comment.
Why do we care whether this is a in the terminal-theme-service? The color is the color of a ThemeIcon in theia.d.ts. That should go through the regular color registry like with the QuickInputItem stuff.
There was a problem hiding this comment.
The code is now using the regular color registry.
|
@tsmaeder |
dhuebner
force-pushed
the
huebner/terminalURI
branch
from
f228b09
to
6f852a8
Compare
|
@dhuebner the license check fails. Do we need an ip ticket here? |
dhuebner
force-pushed
the
huebner/terminalURI
branch
from
54f956b
to
0208720
Compare
dhuebner
force-pushed
the
huebner/terminalURI
branch
from
0208720
to
9b2fb2e
Compare
Partial close of 12074
- This commit adds URI and { light, dark } support for terminal icons.
- This commit adds ColorTheme support for ThemeIcons
- Support for `ExtensionTerminalOptions#iconPath`
Signed-Off-By: FernandoAscencio <fernando.ascencio.cama@ericsson.com>
(cherry picked from commit ba3dda2)
msujew
force-pushed
the
huebner/terminalURI
branch
from
9b2fb2e
to
461c33e
Compare
|
@dhuebner @tsmaeder I've updated the PR myself. I've added support for custom icons for quick pick items and quick pick buttons. You can find an example plugin for that here. Simply use the |
This is a copy of the original PR #12861
First commit was cherry picked from FernandoAscencio@ba3dda2
In the second commit, I applied the suggested changed from the original PR.
The original description below
What it does
Closes #12074
This commit adds URI and { light, dark } support for terminal icons.
This commit adds ColorTheme support for ThemeIcons
Support for ExtensionTerminalOptions#iconPath
How to test
Open Theia
Install from VSIX the test extension
Check the result follow VSCode test results seen below:
Review checklist
Reminder for reviewers