Add support for setting tabColor on the command line #8102

Don-Vito · 2020-10-30T00:21:20Z

Add a tabColor parameter to the new-tab and split-panes command
Add --tabColor to the command line, to allow bootstrapping with tabs
of different colors

Add another field to NewTerminalArgs. Use this field to set
StartingTabColor in Terminal. This color gets overridden by the color
defined by the profile / VT, however can be overridden with the color
picker.

Since the color is the property of the Terminal, when defined for the
tab this color is associated only with the first pane/terminal of the
tab. Additional panes will not inherit this color (to prevent advanced
resolution, where we need to resolve between the inherited color and the
one specified for the pane).

Validation Steps Performed

UT for parameters parsing
Running system with several tabs of different colors.
Adding custom actions with colors
Performing operations like split pane, duplicate and so on

Closes #8075

Don-Vito · 2020-11-02T18:52:07Z

@zadjii-msft - our CI is impossible 😄 All tests past locally but are flaky as hell when running in pipelines.

zadjii-msft

These are kinda just nits, but I do think the Windows::UI::Color thing would be slightly cleaner.

Otherwise, great job here!

src/cascadia/TerminalSettingsModel/ActionArgs.h

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

…r casted to int

Don-Vito · 2020-11-10T11:07:23Z

@zadjii-msft - fixed, please re-review 😊

zadjii-msft

Yea, this is the stuff. Great work!

Don-Vito · 2020-11-13T18:40:09Z

@zadjii-msft, @DHowett - I have created an alternative implementation where I add tabColor only to NewTabArgs. Please let me know if you prefer that variant, or you believe we do need to add tabColor for split-pane. In the latter case I need your help with understanding the expected behavior.

Bump - so I can advance on week-end

DHowett · 2020-11-13T23:11:43Z

So it's interesting. A profile can set a tabColor, which means that panes technically have one. I'll need @zadjii-msft to weigh in.

It might boil down to "it should be like a title -- the focused pane wins -- we just didn't do it like that"

Don-Vito · 2020-11-14T05:04:25Z

So it's interesting. A profile can set a tabColor, which means that panes technically have one. I'll need @zadjii-msft to weigh in.

It might boil down to "it should be like a title -- the focused pane wins -- we just didn't do it like that"

@DHowett - cool. I will try to implement this approach as well and then we can choose 😊

Don-Vito · 2020-11-15T16:39:30Z

@DHowett , @zadjii-msft - changed the implementation to set the color on the pane level.
However, I could not use existing hierarchy:

runtimeTabColor - is not good enough as it is on the tab level
Terminal::TabColor - is not good enough, as it gets overridden upon settings reload

Hence I added a StartingTitle to TermControl, I was not sure if I should add it to TermControl or Terminal itself (I will probably move it to Terminal in a separate PR, and merge to here if required)

Don-Vito · 2020-11-15T20:23:28Z

I decided to move everything to Terminal, we'll see if it was a good decision 😄

Don-Vito · 2020-11-15T21:48:36Z

@zadjii-msft - I added the starting tab color to be a property of the Terminal. Consequently, when a new tab is created the color is set on the first pane. But if we split the tab the color is not set on the second pane. We could say that the color will be applied for all panes of the tab, but this gets complicated, as the color can be also specified on the second pane as well. Which will require us to resolve what color is preferable. This is possible, but I am not sure if we want to get there product-wise. WDYT?

Don-Vito · 2020-11-15T22:28:50Z

This is awesome! Just link the PR for the updated docs and I'll approve. Thanks!

Updated the docs. Thanks!

Don-Vito · 2020-11-18T10:42:38Z

Bumping this as it works pretty cool 😊

zadjii-msft · 2020-11-18T18:25:10Z

Okay, sorry for the delay reviewing this. You're churning out so many dang PRs, it's hard to keep up 😄

I've played with the branch as it is this morning, and I like how it feels. And it feels like what I originally described in #8075. Plus, being resilient to a settings reload is a clever solution.

I'll sanity check the code now, but it definitely feels 👌

zadjii-msft

Oh yea I already reviewed 90% of this. This looks good to me!

carlos-zamora

I guess I'm ok with this. I like the feature. I don't like the TabColor vs StartingTabColor thing, but I'll just have to live with it since it seems to be the best approach tbh.

src/cascadia/TerminalCore/Terminal.cpp

carlos-zamora · 2020-11-18T18:45:37Z

src/cascadia/TerminalApp/TerminalSettings.h

        GETSET_PROPERTY(Windows::Foundation::IReference<uint32_t>, TabColor, nullptr);
+        GETSET_PROPERTY(Windows::Foundation::IReference<uint32_t>, StartingTabColor, nullptr);


@DHowett or @zadjii-msft:
Should we have a comment here or somewhere describing the difference between these two properties? I understand that we need StartingTabColor, but I have a feeling it'll be confusing to have both down the road.

@carlos-zamora - I totally agree with you that it is both confusing and error-prone. Instead I would like to:

Introduce "ITerminalCreationParams" and move this property there.

Pass them upon construction

However this will require changing the Terminal interface, that I felt is somewhat above my paygrade at this stage.

DHowett

This is great. We'll need to get the TerminalSettings back into check because sometimes we use it to communicate app-only state, but I'm happy with this for now.

ghost · 2020-11-20T04:35:43Z

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

* Add a tabColor parameter to the `new-tab` and `split-panes` command * Add --tabColor to the command line, to allow bootstrapping with tabs of different colors Add another field to NewTerminalArgs. Use this field to set StartingTabColor in Terminal. This color gets overridden by the color defined by the profile / VT, however can be overridden with the color picker. Since the color is the property of the Terminal, when defined for the tab this color is associated only with the first pane/terminal of the tab. Additional panes will not inherit this color (to prevent advanced resolution, where we need to resolve between the inherited color and the one specified for the pane). ## Validation Steps Performed * UT for parameters parsing * Running system with several tabs of different colors. * Adding custom actions with colors * Performing operations like split pane, duplicate and so on Closes microsoft#8075

8075: add tabColor to new tab command

c73628f

ghost added Area-Commandline wt.exe's commandline arguments Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Oct 30, 2020

Don-Vito added 2 commits November 2, 2020 18:00

8075: document why we set runtime tab color

c591094

Merge remote-tracking branch 'upstream/main' into fet/8075-color-tab

328227f

8075: protect against empty new terminal args

6caaa43

zadjii-msft self-assigned this Nov 2, 2020

zadjii-msft requested changes Nov 3, 2020

View reviewed changes

src/cascadia/TerminalSettingsModel/ActionArgs.h Outdated Show resolved Hide resolved

src/cascadia/TerminalApp/Resources/en-US/Resources.resw Outdated Show resolved Hide resolved

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 3, 2020

zadjii-msft assigned Don-Vito and unassigned zadjii-msft Nov 3, 2020

Merge remote-tracking branch 'upstream/main' into fet/8075-color-tab

92b732e

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 3, 2020

8075: fix command description in resources file

e7d580b

Don-Vito requested a review from zadjii-msft November 3, 2020 19:50

Don-Vito removed their assignment Nov 5, 2020

Don-Vito added 4 commits November 10, 2020 09:02

Merge remote-tracking branch 'upstream/main' into fet/8075-color-tab

de1db22

8075: adjust the code to TerminalTab

5bce74a

8075: adjust the code to use Windows::UI::Color rather than til::colo…

2f90594

…r casted to int

8075: don't explicitly state tabColor type

dab2b54

zadjii-msft approved these changes Nov 10, 2020

View reviewed changes

zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Nov 10, 2020

ghost requested review from miniksa, carlos-zamora, DHowett, leonMSFT and PankajBhojwani November 10, 2020 12:55

Don-Vito added 2 commits November 15, 2020 15:13

Merge remote-tracking branch 'upstream/main' into fet/8075-color-tab

9e299e1

8075: define tab color on the pane level

8d8210d

Don-Vito requested review from DHowett and zadjii-msft November 15, 2020 16:12

8075: move the handling from TerminalControl to Terminal

f661ac1

8075: perform manual coallescing to avoind changing prototypes

7482e76

Don-Vito requested a review from carlos-zamora November 15, 2020 22:28

zadjii-msft approved these changes Nov 18, 2020

View reviewed changes

zadjii-msft assigned DHowett and carlos-zamora Nov 18, 2020

carlos-zamora approved these changes Nov 18, 2020

View reviewed changes

Don-Vito added 2 commits November 18, 2020 20:56

Merge remote-tracking branch 'upstream/main' into fet/8075-color-tab

9b17157

8075: fix / add missing documentation

55c1c26

DHowett approved these changes Nov 20, 2020

View reviewed changes

DHowett changed the title ~~8075: Set tab color in command line~~ Add support for setting tabColor on the command line Nov 20, 2020

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Nov 20, 2020

ghost merged commit fd37e1d into microsoft:main Nov 20, 2020

Don-Vito deleted the fet/8075-color-tab branch December 3, 2020 17:03

cinnamon-msft mentioned this pull request Jan 22, 2021

Add remaining 1.6 release features MicrosoftDocs/terminal#223

Closed

15 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for setting tabColor on the command line #8102

Add support for setting tabColor on the command line #8102

Don-Vito commented Oct 30, 2020 •

edited by DHowett

Loading

Don-Vito commented Nov 2, 2020 •

edited

Loading

zadjii-msft left a comment

Don-Vito commented Nov 10, 2020

zadjii-msft left a comment

Don-Vito commented Nov 13, 2020

DHowett commented Nov 13, 2020

Don-Vito commented Nov 14, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 18, 2020

zadjii-msft commented Nov 18, 2020

zadjii-msft left a comment

carlos-zamora left a comment

carlos-zamora Nov 18, 2020

Don-Vito Nov 18, 2020 •

edited

Loading

DHowett left a comment

ghost commented Nov 20, 2020

		GETSET_PROPERTY(Windows::Foundation::IReference<uint32_t>, TabColor, nullptr);
		GETSET_PROPERTY(Windows::Foundation::IReference<uint32_t>, StartingTabColor, nullptr);

Add support for setting tabColor on the command line #8102

Add support for setting tabColor on the command line #8102

Conversation

Don-Vito commented Oct 30, 2020 • edited by DHowett Loading

Validation Steps Performed

Don-Vito commented Nov 2, 2020 • edited Loading

zadjii-msft left a comment

Choose a reason for hiding this comment

Don-Vito commented Nov 10, 2020

zadjii-msft left a comment

Choose a reason for hiding this comment

Don-Vito commented Nov 13, 2020

DHowett commented Nov 13, 2020

Don-Vito commented Nov 14, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 15, 2020

Don-Vito commented Nov 18, 2020

zadjii-msft commented Nov 18, 2020

zadjii-msft left a comment

Choose a reason for hiding this comment

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora Nov 18, 2020

Choose a reason for hiding this comment

Don-Vito Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Nov 20, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Don-Vito commented Oct 30, 2020 •

edited by DHowett

Loading

Don-Vito commented Nov 2, 2020 •

edited

Loading

Don-Vito Nov 18, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.