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

A better fix for #tab-titles-are-too-long #2373

Merged
merged 4 commits into from
Aug 14, 2019

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Aug 9, 2019

Summary of the Pull Request

Add a startingTitle setting. This will act just like setting the lpTitle
member of a STARTUPINFO, which is similar to how cmd and powershell being
started from a shortcut works today. cmd and powershell will read their
initial title and keep it around, instead of setting the title like bash
does. When commands in cmd are executed, cmd will append them to the end of
this value, like cmd currently does when started from the "Command Prompt"
shortcut.

Also adds default startingTitles to cmd, powershell, and pwsh.

This is accomplished with a change to conhost.exe to plumb the lpTitle down
to the child process it's starting. This shouldn't necessarily regress
anything, since starting conhost directly and having conhost start a
executable on the caller's behalf is a relatively new and undocumented
scenario.

Here's what it looks like for cmd, powershell, and ubuntu. I set the Ubuntu startingTitle to "Ubuntu", which was immediately overridden when bash started. Cmd is running ping -t github.com, which you can see appended to the tab title like usual.
image

References

literally every issue that mentions the length of the tab title, but #2327 was the one that broke the camel's back.

Also is a way better solution than #2348.

PR Checklist

  • Closes #the-endless-stream-of-complaints-about-tab-titles
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be updated
  • I discussed this with Dustin yesterday. Turns out it was very doable.

  Add a `startingTitle` setting. This will act just like setting the `lpTitle`
  member of a `STARTUPINFO`, which is similar to how cmd and powershell being
  started from a shortcut works today. `cmd` and `powershell` will read their
  initial title and keep it around, instead of setting the title like `bash`
  does. When commands in `cmd` are executed, cmd will append them to the end of
  this value, like cmd currently does when started from the "Command Prompt"
  shortcut.

  Also adds default `startingTitle`s to `cmd`, `powershell`, and `pwsh`.

  This is accomplished with a change to conhost.exe to plumb the `lpTitle` down
  to the child process it's starting. This shouldn't necessarily regress
  anything, since starting conhost directly and having conhost start a
  executable on the caller's behalf is a relatively new and undocumented
  scenario.
@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Aug 9, 2019
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a hundred percent okay with this. Should it be initialTitle? We already have other initial settings, but we also have a startingDirectory . . .

@DHowett-MSFT
Copy link
Contributor

We need to consider what this means for CreatePseudoConsole (CreatePseudoConsoleExW)

@zadjii-msft
Copy link
Member Author

@DHowett-MSFT This will be easy for CreateaPseudoConsole, since we'll be creating the child process.

@@ -33,6 +33,7 @@ Properties listed below are specific to each unique profile.
| `padding` | _Required_ | String | `0, 0, 0, 0` | Sets the padding around the text within the window. Can have three different formats: `"#"` sets the same padding for all sides, `"#, #"` sets the same padding for left-right and top-bottom, and `"#, #, #, #"` sets the padding individually for left, top, right, and bottom. |
| `snapOnInput` | _Required_ | Boolean | `true` | When set to `true`, the window will scroll to the command input line when typing. When set to `false`, the window will not scroll when you start typing. |
| `startingDirectory` | _Required_ | String | `%USERPROFILE%` | The directory the shell starts in when it is loaded. |
| `startingTitle` | Optional | String | | The title to pass to the shell on startup. Some shells (like `bash`) may choose to ignore this initial value, while others (`cmd`, `powershell`) may use this value over the lifetime of the application. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default should be empty string

Should we add a note at the bottom of the doc saying what the defaults are for the generated profiles? Or maybe that's something for a completely different user doc?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also maybe specifying the specific difference between startingTitle and tabTitle?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default is unset, not empty string. Distinction!

@DHowett-MSFT
Copy link
Contributor

I think we should rationalize the names startingTitle and tabTitle before shipping this. I always thought tabTitle should be tabTitleOverride... because it honestly makes it sound like tabTitle could be replaced with this new behavior

Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only nits but i want to think about the naming and implication for existing settings

src/cascadia/TerminalConnection/ConhostConnection.cpp Outdated Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 9, 2019
@zadjii-msft
Copy link
Member Author

@DHowett-MSFT I'm also certainly not sure on the name. I was originally going to go with "shortcutTitle" but that seemed ridiculous when I wrote it - no one outside the console team understands console shortcuts well.

"initialTitle" and "startingTitle" both seem pretty similar to me, but I'm not a PM. Words aren't my job. Summoning @cinnamon-msft and @bitcrazed.

Frankly, until #1320 lands, I think that tabTitle is a strictly worse option than startingTitle, but once #1320 does land, the two might be very powerful in conjunction. I'm going to await some guidance from our PM colleagues on naming here.

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 9, 2019
@DHowett-MSFT
Copy link
Contributor

Controversial: what if we just always push the profile name into Conhost? Maybe we don’t need a new setting for this particular feature. Consider that the profile is the same as the .LNK and previously we used the LNK name as the title?

@DHowett-MSFT
Copy link
Contributor

Oh oh and we then make it so that tabTitle is this setting for if you want it to be different from the profile. We would get rid of the Overriding aspect (which I think is a feature that nobody truly wanted when it was only a static name)?

@zadjii-msft
Copy link
Member Author

@DHowett-MSFT I don't hate the idea of using the profile.name as the starting title. I'd be curious if anyone wanted a separate value for it, because that would be impossible to do with that implementation.

However, I think that tabTitle should maybe stay reserved for #1320, when the overriding will make more sense. Though, we could always add a new setting then if we really wanted.

@zadjii-msft
Copy link
Member Author

zadjii-msft commented Aug 13, 2019

@DHowett-MSFT @carlos-zamora @cinnamon-msft @bitcrazed @miniksa
In preparation for today's later discussion on this topic, I tried to write up scenarios and proposed solutions. @ me if I forgot any scenarios here.

User Stories:

  1. A user wants to be able to use the executable path as their starting title
    • Does anyone want this?
  2. A user wants to be able to set a custom starting title, but have that title be overridable
  3. A user wants to be able to set an overridable starting title, different from the profile name
    • Presumably someone will want this
  4. A user totally wants to ignore the VT title and use something else
    • This will make more sense in the post #1320 "Support runtime variables in the custom user title" settings

Solutions:

  1. name, startingTitle, tabTitle (the PR as it exists currently)
    • a. name is only ever used as the profile name.
    • b. If startingTitle isn't set, then the executable path is used
    • c. If startingTitle is set, it's used as the initial title
    • d. If tabTitle is set, it overrides the title from the terminal
    • e. Current users of tabTitle need to manually update to the new behavior.
  2. name as starting title, tabTitle as a different starting title
    • a. name is used as the starting title and the profile name in the dropdown
    • b. If tabTitle is set, we'll use that as the overridable starting title instead.
    • c. In the future, dynamicTabTitle or tabTitleOverride could be added to support #1320
    • d. Current users of tabTitle automatically get the new (different!) behavior.
    • e. User Story 1 is impossible
      • Does anyone want the behavior ever? Perhaps making that scenario impossible is good?
  3. name unchanged, tabTitle as the starting title
    • a. name is only ever used as the profile name.
    • b. If tabTitle is set, we'll use that as the overridable starting title.
    • c. In the future, dynamicTabTitle or tabTitleOverride could be added to support #1320
    • d. Current users of tabTitle automatically get the new (different!) behavior.
  4. name as starting title, tabTitle as different starting title, suppressApplicationTitle Boolean to force it to override
    • a. name, tabTitle work as in Solution 2.
    • b. When someone wants to be able to statically totally override that title (story 4), they can use suppressApplicationTitle
    • c. suppressApplicationTitle name is WIP
    • d. We'll add suppressApplicationTitle when someone complains
    • e. If you really want story 1, use tabTitle: c:\path\to\foo.exe and suppressApplicationTitle.

We've decided to pursue path 4. I'll update the PR in the morning.

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 14, 2019
@ghost
Copy link

ghost commented Aug 14, 2019

Hello @zadjii-msft!

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.

@DHowett-MSFT
Copy link
Contributor

SA build is failing due to disk space.

@DHowett-MSFT DHowett-MSFT merged commit 82de43b into master Aug 14, 2019
@DHowett-MSFT DHowett-MSFT deleted the dev/migrie/f/starting-title branch August 14, 2019 23:16
@IanKemp
Copy link

IanKemp commented Aug 22, 2019

Dear god I need this. When will you guys be dropping a new release that contains this functionality? Seriously, a release just for this feature would be acceptable at this point.

@ghost
Copy link

ghost commented Aug 27, 2019

🎉Windows Terminal Preview v0.4.2382.0 has been released which incorporates this pull request.:tada:

Handy links:

@Uxorious
Copy link

d. We'll add suppressApplicationTitle when someone complains

Not a complaint, but I would love this :-)

@sth4me
Copy link

sth4me commented Aug 30, 2019

Any option can overrides the title Or just always shows in the tab?When I ssh to a server,I want to know server name I custom.

@thongtrinh2204
Copy link

@zadjii-msft can you please add suppressApplicationTitle feature? I need it on WSL, my tab titles are too long...

@zadjii-msft
Copy link
Member Author

@thongtrinh2204 hey look, it's already in PR #2814

@thongtrinh2204
Copy link

@thongtrinh2204 hey look, it's already in PR #2814

Sounds good. Can wait for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants