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

Windows/tabs created with "wt new-tab COMMAND" start with the title of the default Terminal profile, not the COMMAND name #6776

Closed
metathinker opened this issue Jul 3, 2020 · 7 comments · Fixed by #11029
Assignees
Labels
Area-Commandline wt.exe's commandline arguments Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. this-will-be-a-breaking-change
Milestone

Comments

@metathinker
Copy link
Contributor

Environment

Windows build number: Windows 10 ver. 2004 build 19041.329
Windows Terminal version (if applicable): 1.1.1812.0

Steps to reproduce

  1. While Terminal is closed, delete or rename your settings.json file, causing Terminal to revert to the default settings under which the built-in Windows PowerShell profile for Terminal {61c54bbd-c2c6-5271-96e7-009a87ff44bf} is the default one.
  2. Run this command: wt.exe C:\Windows\System32\robocopy.exe

Expected behavior

The title of the Terminal tab created is "C:\Windows\System32\robocopy.exe" or similar.

Actual behavior

The title of the Terminal tab created is "Windows PowerShell".
(If you watch Task Manager or another process listing, you can see that no PowerShell process was ever started.)

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jul 3, 2020
@DHowett
Copy link
Member

DHowett commented Jul 10, 2020

So, this one is weird... I'm not sure how we should handle it. I'm not certain setting the commandline to be the title is the right thing to do.

A profile is closer to a shell link than an unjacketed command. A traditional console host started from a LNK inherits the stem name of the LNK file, not the commandline string stored inside it. However, there's no true provision for taking a LNK and "running a different command" with it. . . except STARTF_TITLEISLINKNAME. Spawning a console process with one commandline but another [profile equivalent] specified in its startup info results in the same behavior as reported here.

I'm inclined to call it by design. What do you think?

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 10, 2020
@metathinker
Copy link
Contributor Author

metathinker commented Jul 10, 2020

Those are all good points you've made. My point of view, though, is that I'm not specifying a profile name on the wt command line, I'm specifying a command to run. So the analogy between a Terminal profile and a console app shortcut/.lnk file is not something I think of at all. And if I look for the window I just created, I don't expect it to have the title of a shell or app I never actually launched, which is the case now since the profile name is being picked up.

This could be even more of a problem if and when wt from within Terminal is changed to create a new tab instead of a new window, since now you have a new tab in Windows Terminal that is misleadingly named and you won't necessarily be able to see the difference at a glance.

Hence, I would not be happy with a design of Terminal that incorporates the current behavior. We can certainly try to come up with something that we're both happy with, though.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 10, 2020
@DHowett DHowett added Area-Commandline wt.exe's commandline arguments Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 15, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 15, 2020
@DHowett DHowett added this to the Terminal v2.0 milestone Jul 15, 2020
@DHowett DHowett added the Priority-2 A description (P2) label Jul 15, 2020
@zadjii-msft zadjii-msft self-assigned this Jul 13, 2021
@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Jul 13, 2021
@zadjii-msft
Copy link
Member

Okay, we think this is a good idea, especially in the context of defterm, #9458, #10669

@zadjii-msft zadjii-msft removed their assignment Jul 21, 2021
@zadjii-msft
Copy link
Member

zadjii-msft commented Jul 21, 2021

I'm moving this back out to "v2.0". We'll do this in tandem with the change for #10669, because doing that all at once is a good story we can sell. They don't really make sense one at a time.

ALSO

Let's only do this when the commandline is overridden. So wt new-tab will still use the default profile, while wt new-tab foo.exe will use profiles.defaults.

@zadjii-msft zadjii-msft removed Needs-Discussion Something that requires a team discussion before we can proceed Help Wanted We encourage anyone to jump in on these. labels Jul 21, 2021
DHowett added a commit that referenced this issue Aug 23, 2021
Right now, we store GUIDs in panes and most of the functions for interacting
with profiles on the settings model take GUIDs and look up profiles.

This pull request changes how we store and look up profiles to prefer profile
objects. Panes store strong references to their originating profiles, which
simplifies settings lookup for CloseOnExit and the bell settings. In fact,
deleting a pane's profile no longer causes it to forget which CloseOnExit
setting applies to it. Duplicating a pane that is hosting a deleted profile
(#5047) now duplicates the profile, even though it is otherwise unreachable.

This makes the world more consistent and allows us to _eventually_ support panes
hosting profiles that do not have GUIDs that can be looked up in the profile
list. This is a gateway to #6776 and #10669, and consolidating the profile
lookup logic will help with #10952.

PR #10588 introduced TerminalSettings::CreateWithProfile and made
...CreateWithProfileByID a thin wrapper over top it, which looked up the profile
by GUID before proceeding. It has also been removed, as its last caller is gone.

Closes #5047
@DHowett
Copy link
Member

DHowett commented Aug 23, 2021

This was fixed in #10998. Thanks!

@DHowett DHowett closed this as completed Aug 23, 2021
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Aug 23, 2021
@DHowett
Copy link
Member

DHowett commented Aug 23, 2021

(We took the path of making the title match the first segment of the command, and are decoupling profile choice at the same time. We'll work on pulling icons and stuff out if necessary later. Changing the profile is a breaking change, the rest are new fun features.)

DHowett added a commit that referenced this issue Aug 24, 2021
It was insufficient to only promote commandline components to titles
during commandline parsing, because we also have a whole complement of
actions that contain NewTerminalArgs. The tests caught me out a little
too late (sorry!). I decided it was better move promotion down to
TerminalSettings.

Fixes #6776.
@ghost ghost added the In-PR This issue has a related PR label Aug 24, 2021
@ghost ghost removed the In-PR This issue has a related PR label Aug 24, 2021
ghost pushed a commit that referenced this issue Aug 24, 2021
It was insufficient to only promote commandline components to titles
during commandline parsing, because we also have a whole complement of
actions that contain NewTerminalArgs. The tests caught me out a little
too late (sorry!). I decided it was better move promotion down to
TerminalSettings.

Fixes #6776
Re-implements #10998
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Aug 24, 2021
ghost pushed a commit that referenced this issue Aug 25, 2021
This pull request introduces our first use of the "base" profile as an
actual profile. Incoming commandlines from `wt foo` *and* default
terminal handoffs will be hosted in the base profile.

**THIS IS A BREAKING CHANGE** for user behavior.

The original behavior where commandlines were hosted in the "default"
profile (in most cases, Windows PowerShell) led to user confusion: "why
does cmd use my powershell icon?" and "why does the title say
PowerShell?". Making this change unifies the user experience so that we
can land commandline detection in #10952.

Users who want the original behavior can get it back for commandline
invocation by specifying a profile using the `-p` argument, as in `wt -p
PowerShell -- cmd`.

As a temporary stopgap, users who attempt to duplicate the base profile
will get their specified default profile until we land #5047.

This feature is hidden behind the same feature flag that controls the
visibility of base/"Defaults" in the settings UI.

Fixes #10669
Related to #6776
@ghost
Copy link

ghost commented Aug 31, 2021

🎉This issue was addressed in #11029, which has now been successfully released as Windows Terminal Preview v1.11.2421.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Commandline wt.exe's commandline arguments Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. this-will-be-a-breaking-change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants