Add support for commandline args to `wt.exe` #4023

zadjii-msft · 2019-12-19T16:09:02Z

Summary of the Pull Request

Adds support for commandline arguments to the Windows Terminal, in accordance with the spec in #3495

References

Original issue: Feature Request: wt.exe supports command line arguments (profile, command, directory, etc.) #607
Original spec: Add spec for adding commandline arguments to wt.exe #3495

PR Checklist

Closes Feature Request: wt.exe supports command line arguments (profile, command, directory, etc.) #607
I work here
Tests added/passed
We should probably add some docs on these commands
The spec (Add spec for adding commandline arguments to wt.exe #3495) needs to be merged first!

Detailed Description of the Pull Request / Additional comments

🛑 STOP 🛑 - have you read #3495 yet? If you haven't, go do that now.

This PR adds support for three initial sub-commands to the wt.exe application:

new-tab: Used to create a new tab.
split-pane: Used to create a new split.
focus-tab: Moves focus to another tab.

These commands are largely POC to prove that the commandlines work. They're not totally finished, but they work well enough. Follow up work items will be filed to track adding support for additional parameters and subcommands

Important scenarios added:

wt -d .: Open a new wt instance in the current working directory Starting directory no longer respected #878
wt -p <profile name>: Create a wt instance running the given profile, to unblock Feature Request: Task Bar jumplist should show items from profile #576, Draft spec for adding profiles to the Windows jumplist #1357, Feature Request Option to add profiles to the Windows Start Menu #2339
wt ; new-tab ; split-pane -V: Launch the terminal with multiple tabs, splits, to unblock Feature Request: Start with multiple tabs open #756

Validation Steps Performed

Ran tests
Played with it a bunch

…mandlines

… one

…commandline

…tbase! When we startup, we currently have a `Title`, because we always have a Tab. With commandline args, we'll sometimes _not_ have a tab immediately. In that case, this would crash

…mething provided by a commandline

…a new tab

carlos-zamora

Finally got through all of it. Here's a few comments. This thing is awesome!

Don't feel comfortable approving it until I actually download the branch and play with it a bit. I can do that once Seattle Snowpocalypse 2020 ends and I get back into the office (assuming it isn't merged by then).

src/cascadia/WindowsTerminal/AppHost.cpp

src/cascadia/TerminalApp/TerminalPage.cpp

src/cascadia/TerminalApp/AppCommandlineArgs.h

src/cascadia/TerminalApp/AppCommandlineArgs.cpp

carlos-zamora · 2020-01-16T21:05:33Z

Don't feel comfortable approving it until I actually download the branch and play with it a bit. I can do that once Seattle Snowpocalypse 2020 ends and I get back into the office (assuming it isn't merged by then).

Alright, got a chance to play with it. Found a few issues:

I build/deploy your branch. In this terminal ("termA"), I execute command "wtd" in powershell. I get a new terminal instance with the default profile properly. TermA no longer accepts input. I then close the new terminal instance, and this happens...

 	Microsoft.Toolkit.Win32.UI.XamlHost.dll!00007ffd9fcc3e99()	Unknown
>	Windows.UI.Xaml.dll!ctl::release_interface<DirectUI::SettingsFlyoutMetadata>(DirectUI::SettingsFlyoutMetadata * & pInterface) Line 137	C++
 	[Inline Frame] Windows.UI.Xaml.dll!DirectUI::FrameworkApplication::GlobalDeinit() Line 668	C++
 	Windows.UI.Xaml.dll!DeinitializeDll() Line 319	C++
 	Windows.UI.Xaml.dll!DllMain(HINSTANCE__ * hinstDLL, unsigned int fdwReason, void * __formal) Line 394	C++
 	Windows.UI.Xaml.dll!dllmain_dispatch(HINSTANCE__ * const instance, const unsigned long reason, void * const reserved) Line 200	C++
 	ntdll.dll!LdrpCallInitRoutine(unsigned char(*)(void *, unsigned long, _CONTEXT *) InitRoutine, void * DllHandle, unsigned long Reason, _CONTEXT * Context) Line 212	C

When executed from CMD, TermA still accepts input. But upon closing the new terminal instance, the same exception is thrown.

"wtd.exe" does nothing when in wsl (using Ubuntu distro, if it matters)

Is there any way to fix this? ☹

zadjii-msft · 2020-01-16T22:06:25Z

@carlos-zamora:
That crash is a XAML crash that I was supposed to file on the MUX repo. It only happens in debug, and it's been happening for a while now actually 😄

TermA no longer accepts input
wat? Maybe this is just a powershell thing, and it should be wtd &? @DHowett-MSFT thoughts here?

Also, I'm not sure any execution aliases work in wsl. ubuntu.exe doesn't work for me. Not sure there's anything we can do about that.

DHowett-MSFT · 2020-01-16T22:07:57Z

Yes, that’s because powershell 6 cannot detect app execution aliases for their subsystem.

carlos-zamora · 2020-01-24T19:01:36Z

@carlos-zamora:
That crash is a XAML crash that I was supposed to file on the MUX repo. It only happens in debug, and it's been happening for a while now actually 😄

Derp. That's what I get for being in accessibility-land for so long haha.

TermA no longer accepts input
wat? Maybe this is just a powershell thing, and it should be wtd &? @DHowett-MSFT thoughts here?

Also, I'm not sure any execution aliases work in wsl. ubuntu.exe doesn't work for me. Not sure there's anything we can do about that.

But I thought one of the cool things with WSL was that you can just run things like "notepad.exe" or any windows executable and it just works.

DHowett-MSFT · 2020-01-24T19:32:57Z

notepad.exe isn’t an app execution alias. That issue (inbox) pertains only to the little alias files that the app platform emits for applications inside an appx.

DHowett-MSFT

Minor nits, but not enough to blockk

src/cascadia/TerminalApp/AppCommandlineArgs.cpp

DHowett-MSFT · 2020-01-24T22:50:30Z

src/cascadia/TerminalApp/AppCommandlineArgs.cpp

+                if (arg.find(" ") != std::string::npos)
+                {
+                    fullCommandlineBuffer += "\"";
+                    fullCommandlineBuffer += arg;
+                    fullCommandlineBuffer += "\"";
+                }
+                else
+                {
+                    fullCommandlineBuffer += arg;
+                }
+                fullCommandlineBuffer += " ";


big dislike. please file a bug on CLI11 to request a mode that lets us get everything after the first non-parameter string verbatim in the right format.

DHowett-MSFT · 2020-01-24T22:51:39Z

src/cascadia/TerminalApp/AppCommandlineArgs.h

+    // that produces a NewTerminalArgs.
+    struct NewTerminalSubcommand
+    {
+        CLI::App* subcommand;


I don't 100% understand how a subcommand has-a subcommand, but I am willing to accept it

DHowett-MSFT · 2020-01-24T22:51:57Z

src/cascadia/TerminalApp/AppCommandlineArgs.h

    CLI::App* _focusTabCommand;
    // Are you adding a new sub-command? Make sure to update _noCommandsProvided!

+    CLI::Option* _horizontalOption;


these seem unusually global

(to all Args)

…ndle-args

ghost · 2020-01-27T15:34:09Z

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.

ghost · 2020-02-13T18:03:41Z

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

Handy links:

UrbanPotato · 2020-03-18T15:48:02Z

this works around #756 with the following syntax less then ideal but it gets it done
wt -p "Windows Powershell" ; cmd ; ubuntu

shame there is not a good way to simply just tic a box and have this be the default

I should not need to use batch files to launch the terminal thats the entire point of having a new terminal lol

we are circling around this issue really getting support for simply having some default options that don't require digging around in a json file would close three issue and probly more I don't know about

#756
#4601
#4632

I don't think this should wait for post 1.0 this is sorta a killer feature most Linux terminals support being configured as such and it would help push people to use windows terminal which is the end game here

zadjii-msft added 27 commits December 10, 2019 14:59

Add all TerminalApp bits from old branch

1d1e6a6

move existing tests over. AppLogic is now responsible for parsing com…

f1f9ad4

…mandlines

some light cleanup of test code, parsing code

299f435

Add more tests for parsing commandline args

a891b25

Encapsulate Cmdline a bit better ; more tests

703f192

Add a test for escaping delimiters, even if it's not supported yet

9db1d0b

Merge branch 'master' into dev/migrie/f/cli11-commandline

af34e99

Move AppCommandline into two files

c8531ec

trying to find un-escaped delimiters but this seems like it only gets…

1fd1de8

… one

well this gets the tricky tests to pass

dfbecac

woo escaping args correctly!

eba8602

Merge remote-tracking branch 'origin/master' into dev/migrie/f/cli11-…

d10f353

…commandline

doc comments on doc comments

4be7c61

whoops broke this

efbf12c

IMPORTANT! Make sure to use at() here, to avoid the FAILFAST in ucr…

54a3812

…tbase! When we startup, we currently have a `Title`, because we always have a Tab. With commandline args, we'll sometimes _not_ have a tab immediately. In that case, this would crash

Hook up terminal page to some args to test if it can startup doing so…

8bc9e07

…mething provided by a commandline

somehow this works without the super janky chaining

f8c0b74

this works all the way from commandline to application!

ddfc490

add tests for --help command

e367ee1

more tests for commandline args

b05f6a6

when there's an arror message, display a message box

332edf3

add a bunch of comments

c4157bd

make all these added methods camelCase

b8b8cf5

add focus-tab command

dfa056f

when there are no commands provided, assume that we start by opening …

fa53eee

…a new tab

cleanup for review

8f7e442

more cleanup for PR

23f2cd2

zadjii-msft requested review from miniksa, DHowett-MSFT and carlos-zamora December 19, 2019 16:09

This was referenced Jan 13, 2020

Can't run Windows Terminal with parameters #4195

Closed

Allow the user's "open pane" bindings to open a pane with the current pane #1756

Closed

Feature Request: wt.exe supports command line arguments (profile, command, directory, etc.) #607

Closed

Merge branch 'master' into dev/migrie/f/607-handle-args

ac33b9b

carlos-zamora reviewed Jan 15, 2020

View reviewed changes

zadjii-msft added 2 commits January 16, 2020 16:44

Merge branch 'master' into dev/migrie/f/607-handle-args

76a9500

fix the RoOriginate that dustin found

30de44f

zadjii-msft mentioned this pull request Jan 17, 2020

Error message when trying to provide a --startingDirectory to a WSL wt.exe command #4277

Closed

Nits from Carlos, also add some reference documentation

7a14eda

DHowett-MSFT approved these changes Jan 24, 2020

View reviewed changes

zadjii-msft added 2 commits January 27, 2020 08:42

Merge remote-tracking branch 'origin/master' into dev/migrie/f/607-ha…

b54513a

…ndle-args

some more minor nits from dustin

d102bd3

zadjii-msft mentioned this pull request Jan 27, 2020

Enable a vector option to consume all arguments following the first string in the vector CLIUtils/CLI11#417

Closed

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 27, 2020

ghost merged commit 830c22b into master Jan 27, 2020

ghost deleted the dev/migrie/f/607-handle-args branch January 27, 2020 15:34

zadjii-msft mentioned this pull request Jan 28, 2020

Starting directory no longer respected #878

Closed

jlieuw mentioned this pull request Feb 10, 2020

[Feature Request] Allow to pass arguments to preferred shell fork-dev/TrackerWin#504

Closed

zadjii-msft mentioned this pull request Feb 18, 2020

MEGATHREAD: Commandline Args Follow-Up Work #4632

Open

40 tasks

jelster mentioned this pull request Mar 10, 2020

Feature Request: Start teminal when user log-in #2189

Closed

Tyriar mentioned this pull request Mar 24, 2020

Should we use namespacing in the CLI microsoft/vscode#93241

Open

saadanerdetbare mentioned this pull request May 3, 2020

Open as Tab on a existing Terminal window kerol2r20/Windows-terminal-context-menu#17

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for commandline args to `wt.exe` #4023

Add support for commandline args to `wt.exe` #4023

zadjii-msft commented Dec 19, 2019 •

edited

Loading

carlos-zamora left a comment

carlos-zamora commented Jan 16, 2020

zadjii-msft commented Jan 16, 2020

DHowett-MSFT commented Jan 16, 2020 •

edited

Loading

carlos-zamora commented Jan 24, 2020

DHowett-MSFT commented Jan 24, 2020 •

edited

Loading

DHowett-MSFT left a comment

DHowett-MSFT Jan 24, 2020

DHowett-MSFT Jan 24, 2020

DHowett-MSFT Jan 24, 2020

DHowett-MSFT Jan 24, 2020

ghost commented Jan 27, 2020

ghost commented Feb 13, 2020

UrbanPotato commented Mar 18, 2020 •

edited

Loading

Add support for commandline args to wt.exe #4023

Add support for commandline args to wt.exe #4023

Conversation

zadjii-msft commented Dec 19, 2019 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora commented Jan 16, 2020

zadjii-msft commented Jan 16, 2020

DHowett-MSFT commented Jan 16, 2020 • edited Loading

carlos-zamora commented Jan 24, 2020

DHowett-MSFT commented Jan 24, 2020 • edited Loading

DHowett-MSFT left a comment

Choose a reason for hiding this comment

DHowett-MSFT Jan 24, 2020

Choose a reason for hiding this comment

DHowett-MSFT Jan 24, 2020

Choose a reason for hiding this comment

DHowett-MSFT Jan 24, 2020

Choose a reason for hiding this comment

DHowett-MSFT Jan 24, 2020

Choose a reason for hiding this comment

ghost commented Jan 27, 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.

ghost commented Feb 13, 2020

UrbanPotato commented Mar 18, 2020 • edited Loading

Add support for commandline args to `wt.exe` #4023

Add support for commandline args to `wt.exe` #4023

zadjii-msft commented Dec 19, 2019 •

edited

Loading

DHowett-MSFT commented Jan 16, 2020 •

edited

Loading

DHowett-MSFT commented Jan 24, 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.

UrbanPotato commented Mar 18, 2020 •

edited

Loading