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

Maybe add support for move-focus subcommand #6580

Closed
zadjii-msft opened this issue Jun 18, 2020 · 2 comments · Fixed by #8546
Closed

Maybe add support for move-focus subcommand #6580

zadjii-msft opened this issue Jun 18, 2020 · 2 comments · Fixed by #8546
Assignees
@zadjii-msft
Labels
Area-Commandline wt.exe's commandline arguments Issue-Question For questions or discussion 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.6

Comments

@zadjii-msft
Copy link
Member

I'm submitting this one for discussion - I don't think this deserves a spec, but maybe some discussion.

#5464 covers "Add support for focus-pane subcommand". That particular subcommand is tricky, because we'd need to have IDs for Panes, and then that gets into all sorts of discussion like should the ID be for the pane, or the TermControl?.

Since that's hard, I thought I might do something easy like move-focus, which is just the moveFocus action, turned into a wt subcommand. This comes with two caveats:

  • It would be subject to the same bug that moveFocus is, Use visual layout for moveFocusX target #2398.
  • It would behave non-deterministically with auto splits. The commandline might behave differently if a pane splits in a different direction due to a different initial window size. Official guidance would probably be "make sure to specify the split direction for all split-pane commands if you're using move-focus".

@microsoft/windows-console-team Are we okay with shipping this command with the above caveats?

I'm thinking the command is just

  • move-focus [direction]
    where direction is one of left, right, up, down.
@zadjii-msft zadjii-msft added Issue-Question For questions or discussion Product-Terminal The new Windows Terminal. Area-Commandline wt.exe's commandline arguments labels Jun 18, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 18, 2020
@zadjii-msft zadjii-msft mentioned this issue Jun 18, 2020
Open
40 tasks
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 18, 2020
@DHowett
Copy link
Member

DHowett commented Jun 18, 2020

So, when I was talking about how I wanted something more composable than a command that depends on physical location, I wasn't thinking about #4472. I'm not sure it makes sense as something that you can launch with, but... perhaps there's value in it.

@zadjii-msft zadjii-msft mentioned this issue Nov 11, 2020
Open
59 tasks
@zadjii-msft zadjii-msft added this to the Terminal v1.6 milestone Nov 12, 2020
@zadjii-msft zadjii-msft self-assigned this Dec 10, 2020
@zadjii-msft zadjii-msft mentioned this issue Dec 10, 2020
Merged
4 tasks
@ghost ghost added the In-PR This issue has a related PR label Dec 10, 2020
@ghost ghost closed this as completed in #8546 Jan 11, 2021
@ghost ghost removed the In-PR This issue has a related PR label Jan 11, 2021
ghost pushed a commit that referenced this issue Jan 11, 2021
@zadjii-msft
Add a move-focus subcommand (#8546)
7235996 
## Summary of the Pull Request

Adds support for the `move-focus` subcommand to `wt.exe`. This subcommand works _exactly_ like `moveFocus(up|down|left|right)`. 

## References
* Will surely conflict with #8183
* Is goodness even in the world where #5464 exists

## PR Checklist
* [x] Closes #6580 
* [x] I work here
* [x] Tests added/passed
* [x] Docs PR: MicrosoftDocs/terminal#209

## Detailed Description of the Pull Request / Additional comments

Bear with me, I wrote this before paternity leave, so code might be a bit stale.

Oddly, after startup, this _does not_ leave the focus on the pane you moved to. If you `move-focus` during startup, at the end of startup, we'll still focus a _random_ pane. This is because the terminal still auto-focus a TermControl when it's done with layout. While we'll maintain the active control just fine during the startup, at the end of startup, all the controls will complete layout in a random order. 

This is no different than the startup right now. `wt sp ; sp ; sp` will focus a random pane at the end. This is left for a future someone to fix

This is also subject to #2398 / #4692. Moving in a direction isn't _totally_ reliable currently. `focus-pane -t ID` will certainly be more reliable, but this will work in the meantime?

## Validation Steps Performed

Opened probably 100 terminals, confirmed that the layout was always correct. Final focused pane was random, but the layout was right.
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jan 11, 2021
@ghost
Copy link

ghost commented Jan 28, 2021

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

Handy links:

mpela81 pushed a commit to mpela81/terminal that referenced this issue Jan 28, 2021
@zadjii-msft @mpela81
Add a move-focus subcommand (microsoft#8546)
4bb6777 
## Summary of the Pull Request

Adds support for the `move-focus` subcommand to `wt.exe`. This subcommand works _exactly_ like `moveFocus(up|down|left|right)`. 

## References
* Will surely conflict with microsoft#8183
* Is goodness even in the world where microsoft#5464 exists

## PR Checklist
* [x] Closes microsoft#6580 
* [x] I work here
* [x] Tests added/passed
* [x] Docs PR: MicrosoftDocs/terminal#209

## Detailed Description of the Pull Request / Additional comments

Bear with me, I wrote this before paternity leave, so code might be a bit stale.

Oddly, after startup, this _does not_ leave the focus on the pane you moved to. If you `move-focus` during startup, at the end of startup, we'll still focus a _random_ pane. This is because the terminal still auto-focus a TermControl when it's done with layout. While we'll maintain the active control just fine during the startup, at the end of startup, all the controls will complete layout in a random order. 

This is no different than the startup right now. `wt sp ; sp ; sp` will focus a random pane at the end. This is left for a future someone to fix

This is also subject to microsoft#2398 / microsoft#4692. Moving in a direction isn't _totally_ reliable currently. `focus-pane -t ID` will certainly be more reliable, but this will work in the meantime?

## Validation Steps Performed

Opened probably 100 terminals, confirmed that the layout was always correct. Final focused pane was random, but the layout was right.
This issue was closed.
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-Question For questions or discussion 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.6
Development

Successfully merging a pull request may close this issue.

Add a move-focus subcommand microsoft/terminal
2 participants
@DHowett @zadjii-msft