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

Add more CLI flags #186

Closed
nikitabobko opened this issue Mar 21, 2024 · 7 comments
Closed

Add more CLI flags #186

nikitabobko opened this issue Mar 21, 2024 · 7 comments
Labels
feature-proposal A well defined feature proposal

Comments

@nikitabobko
Copy link
Owner

(1) Add --window-id option to all or almost all commands. This option would make the command behave like if <window-id> was focused

Motivation:

  • Under the hood --window-id is already presented. That's how on-window-detected.run is implemented. We could make it more transparent by saying in the docs that --window-id is applied by default to all of the on-window-detected.run commands. Lift limitations in on-window-detected.run #20
  • It's another way to fix Feature Request: Add command to focus window by id #173 . We could say that if no args were passed to focus command then we force focus the current window. This way focusing the window by id becomes aerospace --window-id <ID> focus

(2) Add --no-follow-focus to move and --follow-focus to move-node-to-workspace commands (btw, it's inconsistent that move follows focus by default, but move-node-to-workspace doesn't).

Motivation:

  • It makes scripting easier. Yes, it's possible to write aerospace move-node-to-workspace <workspace> && aerospace workspace <workspace>, but it gets more problematic when move-node-to-workspace is invoked by xargs. "Follow focus" is quite a common use case, it would be nice to have a flag for it.
  • We could lift the second limitation from Lift limitations in on-window-detected.run #20 by saying that --follow-focus is applied by default to move-node-to-workspace

(3) Add --format window-id (or --window-id-only) to list-windows command.

Motivation: It's tedious and error-prone to write aerospace list-windows | awk '{print $1}'.

In the same way, --format monitor-id (or --monitor-id-only) can be added to list-monitors command

Motivation for all 3 flags:

(a) Move all windows to the current workspace #185:

focused=$(aerospace list-workspaces --focused)
aerospace list-windows --all --format window-id | xargs -n1 aerospace move-node-to-workspace $focused --follow-focus --window-id
@nikitabobko nikitabobko added feature-proposal A well defined feature proposal design-needed labels Mar 21, 2024
@nikitabobko nikitabobko mentioned this issue Mar 21, 2024
Open
@nikitabobko
Copy link
Owner Author

--follow-focus

It's more correct to name it --focus-follows-window

@sbarrios93 sbarrios93 mentioned this issue Apr 3, 2024
Closed
@nikitabobko nikitabobko mentioned this issue Apr 24, 2024
Closed
@nikitabobko
Copy link
Owner Author

nikitabobko commented Apr 26, 2024 •
edited
Loading

I'm thinking out loud

--window-id is a bad name. Since the flag affects a lot of commands, it should have a more distinct, verbosish name; otherwise, it could "occupy" a flag name that commands might want to use for their own purposes. For example, the focus command now occupied its own --window-id flag which is not exactly the same as the proposed --window-id in this issue

Another observation is that an empty workspace might be the "subject" as well. The workspace flag needs to be named with a symmetrical name

Proposals:

  • --act-on-window-id <subject-window-id>/--act-on-empty-workspace <subject-workspace-name>
  • --subject-window-id <subject-window-id>/--subject-empty-workspace <subject-workspace-name>
  • --src-window-id <subject-window-id>/--src-empty-workspace <subject-workspace-name>

Description proposal: Make the command acts as if the <subject-window-id> or <subject-workspace-name> was focused before the command execution

Other things that need to be thought about are:

  • Feature interaction with the future swap command Implement swap command #8. The command has two "subjects" it acts on. Are there any other potential commands like that?
  • Feature interaction with move-node-to-workspace. What window should be focused by the end of the command if the --subject-window-id flag is used?

@nick-potts
Copy link

How about --target-window and --target-workspace

@nikitabobko nikitabobko mentioned this issue Jun 21, 2024
Closed
@nikitabobko
Copy link
Owner Author

To make the flags even more useful move-node-to-workspace (and maybe move-node-to-monitor) should add support for focused argument:

USAGE: move-node-to-workspace [-h|--help] [--wrap-around] (next|prev)
   OR: move-node-to-workspace [-h|--help] focused
   OR: move-node-to-workspace [-h|--help] <workspace-name>

#299

@nikitabobko nikitabobko mentioned this issue Jun 28, 2024
Closed
nikitabobko added a commit that referenced this issue Jul 20, 2024
@nikitabobko
Decouple focus state management from mostRecentWindow
c066b2a 
Previously, Workspace.mostRecentWindow was the source of focus. This
commit introduces a separate 'focus: LiveFocus' property that is
responsible to track the focused window/emptyWorkspace.

Soon will become possible to do things like 'move-node-to-workspace --window-id 42 focused'
(#186)
Since I don't want this command changing the focus on the workspace,
we need to decouple focus state management from mostRecentWindow

This commit breaks the following tests:
- Test Case '-[AppBundleTests.FocusCommandTest testFocusWrapping]' failed (0.000 seconds).
- Test Case '-[AppBundleTests.FlattenWorkspaceTreeCommandTest testSimple]' failed (0.004 seconds).

I will fix the tests in the next commits
This was referenced Jul 21, 2024
Closed
Closed
@MarcinDominiak
Copy link

Thanks for pointing me here, do you have a time horizon in mind for those changes? I'm quite eagar to start using move-workspace-to-monitor --workspace <workspace> focused, and while this is outside my area of expertise I think I might be able to contribute it in a PR.
Does that sound good to you @nikitabobko or do you want to flesh out the design/have an implementation planned already?

@nikitabobko
Copy link
Owner Author

and while this is outside my area of expertise I think I might be able to contribute it in a PR

I prefer to it implement it myself. The proper implementation requires non-trivial rewrites around CommandSubject and CommandMutableState.

Though nobody can stop you from forking and implementing your own ad-hoc solution until the proper implementation is ready.

do you have a time horizon in mind for those changes?

Everything in this project is ready when it's ready

@nikitabobko nikitabobko mentioned this issue Aug 11, 2024
Closed
jakenvac pushed a commit to jakenvac/AeroSpace that referenced this issue Aug 16, 2024
@nikitabobko
Decouple focus state management from mostRecentWindow
cacf6f6 
Previously, Workspace.mostRecentWindow was the source of focus. This
commit introduces a separate 'focus: LiveFocus' property that is
responsible to track the focused window/emptyWorkspace.

Soon will become possible to do things like 'move-node-to-workspace --window-id 42 focused'
(nikitabobko#186)
Since I don't want this command changing the focus on the workspace,
we need to decouple focus state management from mostRecentWindow

This commit breaks the following tests:
- Test Case '-[AppBundleTests.FocusCommandTest testFocusWrapping]' failed (0.000 seconds).
- Test Case '-[AppBundleTests.FlattenWorkspaceTreeCommandTest testSimple]' failed (0.004 seconds).

I will fix the tests in the next commits
@nikitabobko nikitabobko mentioned this issue Aug 30, 2024
Closed
3 tasks
@Petemir Petemir mentioned this issue Sep 3, 2024
Closed
nikitabobko added a commit that referenced this issue Sep 21, 2024
@nikitabobko
Add windowId and workspace properties to all CmdArgs
c078a63 
#186
nikitabobko added a commit that referenced this issue Sep 21, 2024
@nikitabobko
Rewrite. Split CommandMutableState to more granular CmdIo, CmdEnv and…
61594ee 
… CmdResult

This commit lays out the foundation for the following issues
- #186
- #278
- #20

Also the commit changes how focused window is tracked throught the chain
of executed commands.

- CommandMutableState was a mutable state that tracked the focused
  window, and the track had to be updated throught the chain of executed
  commands (for example, take a look at the `focus` command)
- CmdEnv is simplier. It just forces a particular window to be percieved
  as "focused".

CmdEnv approach makes things easier to understand and describe in the
docs (which I'm going to do later, CmdEnv will be exposed as
AEROSPACE_FOCUSED_WORKSPACE and AEROSPACE_FOCUSED_WINDOW_ID environment
variables)

Unlike CommandMutableState, CmdEnv approach disallows to change focused
window in on-window-detected, on-focus-changed, and other callbacks.
Which I think is not an issue at all. It maybe even considered a safety
mechanism.

If a user uses `close` in one of the mentioned callbacks, previously, a
random window could become focused and it would receive all the
following commands. Now, all the commands that go after `close` will
fail with "Invalid <window-id> \(windowId) specified in
AEROSPACE_FOCUSED_WINDOW_ID env variable"

- This commit is not a breaking change for on-window-detected thanks to
  limitations in #20
- But this commit is technically a breaking change for other mentioned
  callbacks, since no limitations were impoosed on those callbacks. But
  I don't believe that anyone in sane mind relied on it. And the docs
  are explicit that changing focus in those callbacks is a bad idea:

  > Changing the focus within these callbacks is a bad idea anyway, and the way it’s handled will probably change in future versions.

Currently, the "force focused state" in CmdEnv is immutable, and I hope
it will stay so. But hypothetically, it can be mutable in future
versions if we decide that the embedded language #278 should allow
chaning environment variables.
@nikitabobko nikitabobko mentioned this issue Sep 23, 2024
Closed
3 tasks
nikitabobko added a commit that referenced this issue Sep 23, 2024
@nikitabobko
Add --window-id and --workspace flags to all commands where it makes …
47209bd 
…sense

#186
nikitabobko added a commit that referenced this issue Sep 23, 2024
@nikitabobko
move-node-to-workspace. Implement --focus-follows-window
6ea3b1f 
#186
nikitabobko added a commit that referenced this issue Oct 13, 2024
@nikitabobko
Implement summon-workspace command
2fcf32d 
_fixes #338
#186

The command is a replacement for planned
`move-workspace-to-monitor --workspace <workspace> focused`.
The problem with `move-workspace-to-monitor` is that, unlike
`move-workspace-to-monitor next|prev`, it changes the focused workspace.

That's why I think it should be a separate command with its own semantic
nikitabobko added a commit that referenced this issue Oct 13, 2024
@nikitabobko
Implement summon-workspace command
43e6c6b 
_fixes #338
#186

The command is a replacement for planned
`move-workspace-to-monitor --workspace <workspace> focused`.
The problem with `move-workspace-to-monitor` is that, unlike
`move-workspace-to-monitor next|prev`, it changes the focused workspace.

That's why I think it should be a separate command with its own semantic
@nikitabobko
Copy link
Owner Author

Released in 0.15.0-Beta

To make the flags even more useful move-node-to-workspace (and maybe move-node-to-monitor) should add support for focused argument:

I decided to extract it to a separate summon-workspace command

This was referenced Oct 26, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-proposal A well defined feature proposal
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nick-potts @MarcinDominiak @nikitabobko