-
-
Notifications
You must be signed in to change notification settings - Fork 22
api: Add option to swap a whole tag to another output in one go. #392
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Just my 2 cents (you might want to wait for ottatop input on this): Could you add the lua side as well ? |
8e46a66 to
f17cb71
Compare
eb36df7 to
312a70d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks ! Lua side looks good !
(I can't mark the comments as resolved, feel free to do so)
Ottatop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far!
I'd like this feature to be named move_to_output seeing as it moves tags from one output to another.
Also, this currently doesn't have any special handling for windows with multiple tags. If you move a tag, some windows may end up with tags on two different outputs, which will mess with layouting. I can see two options here:
- Remove the moved tag from windows that have it and at least one other tag, or
- Remove all tags except the moved tag on all windows with that tag.
I guess just add a boolean flag to let people choose which behavior they want, though input on how this API should be shaped is welcome.
api/lua/pinnacle/tag.lua
Outdated
| --- | ||
| ---@param output pinnacle.output.OutputHandle The output to add tags to. | ||
| ---@param tags pinnacle.tag.TagHandle The names of the new tags. | ||
| function tag.switch_output(output, tags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of being a function in this module, I'd like this to be a method on TagHandle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ottatop could we have both ?
IMO the module level function is useful if an output disconnect and you want to retrieve some or all tags.
With the module level function, you can simply call OutputHandle:tags() and forward its return value directly (or after filtering the array)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I suppose with the module-level function you could move multiple tags at once and windows with all those tags wouldn't have any removed. Yea, we can have both.
api/rust/src/tag.rs
Outdated
| /// let tag_to_move = tag::get("1")?; | ||
| /// tag::switch_output(&output, tag_to_move); | ||
| /// ``` | ||
| pub fn switch_output<I>(output: &OutputHandle, tag_handles: I) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as lua comment above
312a70d to
a025000
Compare
… a utility function on a single handle
ea8e5bd to
efc37c0
Compare
No description provided.