Allow configuring suppressApplicationTitle in new tab/pane/window commands#9392
Conversation
ghost
added
Area-Commandline
carlos-zamora
left a comment
carlos-zamora
left a comment
There was a problem hiding this comment.
Awesome! Great work (as always haha)!
| _startingTabColor, | ||
| RS_A(L"CmdTabColorArgDesc")); | ||
|
|
||
| subcommand.suppressApplicationTitleOption = subcommand.subcommand->add_flag( |
There was a problem hiding this comment.
Should we add some kind of shorthand version of this? suppressApplicationTitle is one of the longest named settings we have, I think haha.
There was a problem hiding this comment.
I am not sure, I usually write those once (in startupActions 😛 )
There was a problem hiding this comment.
Yea, I say let's leave it for now, and if someone really wants a single-character flag for it in the future, and has a good idea, then we can address it then.
| } | ||
| if (newTerminalArgs.SuppressApplicationTitle()) | ||
| { | ||
| settings.SuppressApplicationTitle(newTerminalArgs.SuppressApplicationTitle().Value()); |
There was a problem hiding this comment.
This is totally off-topic to this PR, but something I wanted to discuss out loud. Since we get the profile and generate a TerminalSettings from it, that means we get base layer (or profiles.defaults) applied properly. Maybe we should emphasize this in the docs? I feel like most people that use this setting want to enable it on every profile, but that's just a gut feeling.
There was a problem hiding this comment.
Actually I am not sure.. I usually use two types of tabs:
- General purpose ones, where I do change folders and stuff - I want the title to change
- Dedicated ones (e.g., git bash in terminal folder) that I don't - I want the title to remain Terminal Dev.
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
Okay so my only concern before approving this:
right now you can pass --suppressApplicationTitle, and that'll set NewTerminalArgs::suppressApplicationTitle to true. Do we need something like --useApplicationTitle that'll force NewTerminalArgs::suppressApplicationTitle to false?
|
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
wow I did not know about that in CLI11. TIL!
from the docs for future me:
Flag options specified through the
add_flag*functions allow a syntax for the option names to default particular options to a false value or any other value if some flags are passed. For example:app.add_flag("--flag,!--no-flag",result,"help for flag");specifies that if
--flagis passed on the command line result will be true or contain a value of 1. If--no-flagis
passedresultwill contain false or -1 ifresultis a signed integer type, or 0 if it is an unsigned type. An
alternative form of the syntax is more explicit:"--flag,--no-flag{false}"; this is equivalent to the previous
example. This also works for short form options"-f,!-n"or"-f,-n{false}". Ifvariable_to_bind_tois anything but an integer value the
default behavior is to take the last value given, while ifvariable_to_bind_tois an integer type the behavior will be to sum
all the given arguments and return the result.
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! Because this pull request has the 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 (
|
ghost
merged commit
|
🎉 Handy links: |
ghost
mentioned this pull request
3 participants
PR Checklist
suppressApplicationTitle#9345Detailed Description of the Pull Request / Additional comments
Introduce optional
suppressApplicationTitlein toNewTerminalArgs.When set (either to true or false) overrides profile configuration.
Introduce
--suppressApplicationTitleflag to command line arguments.When provided for sub=command,
sets the value in the relevant
NewTerminalArgstotrue