Add a move-focus subcommand
#8546
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hort-command-names
…aybe-move-focus # Conflicts: # src/cascadia/TerminalApp/AppCommandlineArgs.cpp # src/cascadia/TerminalApp/AppCommandlineArgs.h
ghost
added
Area-Commandline
# Conflicts: # src/cascadia/TerminalApp/AppCommandlineArgs.h # src/cascadia/TerminalApp/Resources/en-US/Resources.resw
…m _before_ opening the PR
zadjii-msft
added a commit
to MicrosoftDocs/terminal
that referenced
this pull request
Dec 15, 2020
DHowett
reviewed
Dec 15, 2020
DHowett
reviewed
Dec 15, 2020
DHowett
reviewed
Dec 15, 2020
DHowett
approved these changes
Dec 15, 2020
Don-Vito
reviewed
Dec 16, 2020
| void AppCommandlineArgs::_buildMoveFocusParser() | ||
| { | ||
| _moveFocusCommand = _app.add_subcommand("move-focus", RS_A(L"CmdMoveFocusDesc")); | ||
| _moveFocusShort = _app.add_subcommand("mf", RS_A(L"CmdMFDesc")); |
There was a problem hiding this comment.
@zadjii-msft - dumb question.. let's say we introduce "m" for minimized, and then I write -mf. What will the command line do: minimized focus or move focus? 😄
There was a problem hiding this comment.
Presumably, wt -mf sp ; mf left will still work, because the --minimizeFocus arg will be an option, not a subcommand. I'd think that CLI11 should be able to differentiate between the -mf and mf args. If not, that would definitely be an upstream bug 😄
Already done, on L532 😄 |
ghost
requested review from
miniksa,
carlos-zamora,
leonMSFT and
PankajBhojwani
Yeah. Weird - I checked few times, as I was surprised you will forget after doing so few days ago for focus tab. In any case it is there and looks lovely. |
carlos-zamora
approved these changes
Jan 8, 2021
Co-authored-by: Carlos Zamora <[email protected]>
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! Because this pull request has the 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 (
|
15 tasks
|
🎉 Handy links: |
ghost
mentioned this pull request
mpela81
pushed a commit
to mpela81/terminal
that referenced
this pull request
Jan 28, 2021
## 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 pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Adds support for the
move-focussubcommand towt.exe. This subcommand works exactly likemoveFocus(up|down|left|right).References
focus-panesubcommand #5464 existsPR Checklist
move-focussubcommand #6580move-focussubcommand MicrosoftDocs/terminal#209Detailed 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-focusduring 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 ; spwill focus a random pane at the end. This is left for a future someone to fixThis is also subject to #2398 / #4692. Moving in a direction isn't totally reliable currently.
focus-pane -t IDwill 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.