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.

Sign up for GitHub

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

Jump to bottom

wt command-line cannot consistently parse more than one argument #4618

Closed
Jaykul opened this issue Feb 16, 2020 · 9 comments · Fixed by #5090
Closed

wt command-line cannot consistently parse more than one argument #4618

Jaykul opened this issue Feb 16, 2020 · 9 comments · Fixed by #5090
Assignees
@zadjii-msft
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-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.0

Comments

@Jaykul
Copy link
Contributor

Jaykul commented Feb 16, 2020

Bottom line: the more arguments I put on wt the less likely it is to work. This is currently unusable even for launching the app clean.

Something like this, in a run dialog:

wt new-tab -p "PowerShell Core"; split-pane -H -p "Windows PowerShell"; split-pane -p cmd

Seems to work less than one time in five on my computer. Most of the time I get two panes, not three, and occasionally only one.

Environment

Windows build number: Microsoft Windows [Version 10.0.18363.657]
Windows Terminal Version: 0.9.433.0

Steps to reproduce

Run the command above from a run dialog a half dozen times

Expected behavior

The window should open the same way every time, with three panes, one for each of the shells I have in Windows...

Actual behavior

Sometimes just PowerShell Core, sometimes both PowerShells, rarely all three including cmd.
@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 Feb 16, 2020
@Jaykul Jaykul mentioned this issue Feb 16, 2020
Closed
@opus131

This comment has been minimized.

Sign in to view
@zadjii-msft zadjii-msft mentioned this issue Feb 18, 2020
Open
40 tasks
@zadjii-msft zadjii-msft added the Product-Terminal The new Windows Terminal. label Feb 18, 2020
@wrexxor

This comment has been minimized.

Sign in to view
@DHowett-MSFT

This comment has been minimized.

Sign in to view
@DHowett-MSFT DHowett-MSFT added Area-Commandline wt.exe's commandline arguments and removed Product-Terminal The new Windows Terminal. labels Feb 18, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Feb 18, 2020
@DHowett-MSFT DHowett-MSFT added Issue-Bug It either shouldn't be doing this or needs an investigation. Help Wanted We encourage anyone to jump in on these. Product-Terminal The new Windows Terminal. labels Feb 18, 2020
@cinnamon-msft cinnamon-msft added Priority-2 A description (P2) Priority-1 A description (P1) v1-Scrubbed and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-2 A description (P2) labels Feb 19, 2020
@moswald
Copy link
Member

moswald commented Feb 24, 2020

I can confirm that wt does something similar on my machine. Repeating the following command: wt ; split-pane -V ; split-pane -V has resulted in:

  • Two vertical panes, with the right-most vertical pane also split horizontally (what?)
  • Two vertical panes
  • No panes, just the default tab

@brucejo75
Copy link

brucejo75 commented Mar 2, 2020 •
edited
Loading

I am seeing the same thing.

Repro

It repros with a very simple scenario...
Execute this multiple times and often times the last split-pane does not execute:

wt.exe new-tab; split-pane -V ;split-pane -H

Note: getting rid of the last command seems to always work:

wt.exe new-tab; split-pane -V
  -or-
wt.exe new-tab; split-pane -H

@ranyitz ranyitz mentioned this issue Mar 3, 2020
Open
@nbabanov
Copy link

nbabanov commented Mar 9, 2020

I also reproduce it.

@3s-217
Copy link

3s-217 commented Mar 15, 2020

split-pane ,new-tab only works from Run, address bar and search bar, but does not work if executed from powershell or cmd

@zadjii-msft zadjii-msft self-assigned this Mar 18, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 18, 2020
This was referenced Mar 20, 2020
Closed
Merged
@ghost ghost added the In-PR This issue has a related PR label Mar 23, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Mar 26, 2020
DHowett-MSFT pushed a commit that referenced this issue Mar 26, 2020
@zadjii-msft
Process actions sync. on startup; don't dupe nonexistent profile (#5090)
b3fa88e 
This PR has evolved to encapsulate two related fixes that I can't really
untie anymore.

#2455 - Duplicating a tab that doesn't exist anymore

This was the bug I was originally fixing in #4429. 

When the user tries to `duplicateTab` with a profile that doesn't exist
anymore (like might happen after a settings reload), don't crash.

As I was going about adding tests for this, got blocked by the fact that
the Terminal couldn't open _any_ panes while the `TerminalPage` was size
0x0. This had two theoretical solutions: 

* Fake the `TerminalPage` into thinking it had a real size in the test -
  probably possible, though I'm unsure how it would work in practice.
* Change `Pane`s to not require an `ActualWidth`, `ActualHeight` on
  initialization. 

Fortuately, the second option was something else that was already on my
backlog of bugs. 

#4618 - `wt` command-line can't consistently parse more than one arg

Presently, the Terminal just arbitrarily dispatches a bunch of handlers
to try and handle all the commands provided on the commandline. That's
lead to a bunch of reports that not all the commands will always get
executed, nor will they all get executed in the same order. 

This PR also changes the `TerminalPage` to be able to dispatch all the
commands sequentially, all at once in the startup. No longer will there
be a hot second where the commands seem to execute themselves in from of
the user - they'll all happen behind the scenes on startup. 

This involved a couple other changes areound the `TerminalPage`
* I had to make sure that panes could be opened at a 0x0 size. Now they
  use a star sizing based off the percentage of the parent they're
  supposed to consume, so that when the parent _does_ get laid out,
  they'll take the appropriate size of that parent.
* I had to do some math ahead of time to try and calculate what a
  `SplitState::Automatic` would be evaluated as, despite the fact that
  we don't actually know how big the pane will be. 
* I had to ensure that `focus-tab` commands appropriately mark a single
  tab as focused while we're in startup, without roundtripping to the
  Dispatcher thread and back

## References

#4429 - the original PR for #2455
#5047 - a follow-up task from discussion in #4429
#4953 - a PR for making panes use star sizing, which was immensly
        helpful for this PR.

## Detailed Description of the Pull Request / Additional comments

`CascadiaSettings::BuildSettings` can throw if the GUID doesn't exist.
This wraps those calls up with a try/catch.

It also adds a couple tests - a few `SettingsTests` for try/catching
this state. It also adds a XAML-y test in `TabTests` that creates a
`TerminalPage` and then performs som UI-like actions on it. This test
required a minor change to how we generate the new tab dropdown - in the
tests, `Application::Current()` is _not_ a `TerminalApp::App`, so it
doesn't have a `Logic()` to query. So wrap that in a try/catch as well.

While working on these tests, I found that we'd crash pretty agressively
for mysterious reasons if the TestHostApp became focused while the test
was running. This was due to a call in
`TSFInputControl::NotifyFocusEnter` that would callback to
`TSFInputControl::_layoutRequested`, which would crash on setting the
`MaxSize` of the canvas to a negative value. This PR includes a hotfix
for that bug as well. 

## Validation Steps Performed

* Manual testing with a _lot_ of commands in a commandline
* run the tests
* Team tested in selfhost

Closes #2455
Closes #4618
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR labels Mar 26, 2020
@DHowett-MSFT DHowett-MSFT mentioned this issue Apr 3, 2020
Closed
@ghost
Copy link

ghost commented Apr 22, 2020

🎉This issue was addressed in #5090, which has now been successfully released as Windows Terminal Preview v0.11.1121.0.:tada:

Handy links:

@nbabanov
Copy link

It works properly now! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

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-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Process actions sync. on startup; don't dupe nonexistent profile microsoft/terminal
10 participants
@moswald @Jaykul @opus131 @nbabanov @wrexxor @DHowett-MSFT @brucejo75 @zadjii-msft @3s-217 @cinnamon-msft