Extend swap command to support workspaces #7907

fabianscode · 2024-01-12T15:08:54Z

This extends the swap command to also support workspace swapping.
It solves an issue a friend and me had in sway, which is swapping the
contents of two workspaces efficiently.

The syntax goes as follows:
swap workspace with [number] <name>

It will swap the content of the specified workspace with the content of
the currently focused workspace. The currently focused workspace stays
focused. If the other workspace doesnt exist, it will be created,
otherwise if the other workspace is empty afterwards, it will potentially
be destroyed.

It was achieved by simpy swapping relevant fields of the workspaces
and updating a few variables of the affected containers.

The added command allow the user to swap the contents of two workspaces. Co-authored-by: Arne <[email protected]>

This will have the function stop the recusive calls as soon as the first view is found - unlike before

…kspace

bonsaiiV · 2024-02-05T14:34:51Z

Why add an entirely new command? The swap command already exists and intuitively it should work on the content of workspaces too.

To me it feels more like there is an oversight with swap, that needs to be fixed, as it is not working with workspaces.

simonfogliato · 2024-02-06T18:56:47Z

I have been looking for a nice way to swap all windows from one workspace (on monitor one), to a second workspace (on monitor two). The swayr project looks promising but I'd love to see more stuff like this in native sway.

https://sr.ht/~tsdh/swayr/#menu-switchers

At the moment I have been using (swaymsg -t get_outputs) the following...

bindsym $mod+m output "HDMI-A-1" toggle
bindsym $mod+Control+Shift+Right move workspace to output right
bindsym $mod+Control+Shift+Left move workspace to output left
bindsym $mod+Control+Shift+Down move workspace to output down
bindsym $mod+Control+Shift+Up move workspace to output up

fabianscode · 2024-02-07T12:09:30Z

Why add an entirely new command? The swap command already exists and intuitively it should work on the content of workspaces too.

To me it feels more like there is an oversight with swap, that needs to be fixed, as it is not working with workspaces.

I guess that seems reasonable... any other opinions?

cartok · 2024-02-22T07:46:41Z

I have a script on my Todo list as a workaround and think this is really a good feature, especially when working with more than one screen.

Co-authored-by: Arne <[email protected]>

fabianscode · 2024-02-26T17:57:39Z

Why add an entirely new command? The swap command already exists and intuitively it should work on the content of workspaces too.

To me it feels more like there is an oversight with swap, that needs to be fixed, as it is not working with workspaces.

the latest commit implements exactly that. Any thoughts?

bonsaiiV

This swaps the workspaces entirely. I was thinking about just inserting a container and swapping that, in case a workspace is selected to swap.
That's some different functionality.

Overall this approach looks fine. And basic testing showed, that it works.
I like it and think it is a nice addition. Maintainers seem reluctant about adding new stuff though. (to keep i3 compat)

Issues commented should get fixed though.

bonsaiiV · 2024-02-27T19:22:46Z

sway/sway.5.scd

@@ -344,14 +344,20 @@ set|plus|minus|toggle <amount>
 	"Sticks" a floating window to the current output so that it shows up on all
 	workspaces.



Keep the old documentation and document the new syntax just below. That would just be simpler.

The documentation of the new syntax is also very short :

what happens to container focus

how is the argument specified (e.g. workspace with the number)

bonsaiiV · 2024-02-27T19:44:28Z

include/sway/commands.h

I think to add that command you also need to link a function to that command in commands.c,
forgetting this here might break stuff

bonsaiiV · 2024-02-27T19:49:12Z

sway/commands/swap.c

+	if (!oth_ws) {
+		oth_ws = workspace_create(NULL, ws_name);
+
+		if (!oth_ws) {


i think here is free(ws_name) missing and potentially leaking memory

bonsaiiV · 2024-02-27T19:50:57Z

sway/commands/swap.c

+
+	// second workspace is the one currently focused
+	struct sway_workspace *cur_ws = config->handler_context.workspace;
+	if (!cur_ws) {


exiting here will potentially leave oth_ws created, if it wasn't before

bonsaiiV · 2024-02-27T21:23:39Z

sway/commands/swap.c

@@ -1,7 +1,9 @@
 #include <strings.h>


is this ever used?

bonsaiiV · 2024-02-27T21:30:19Z

sway/commands/swap.c

+
+struct cmd_results *cmd_swap(int argc, char **argv) {
+	struct cmd_results *error = NULL;
+	if ((error = checkarg(argc, "swap", EXPECTED_AT_LEAST, 4))) {


this does not allow to identify the workspace by name as that uses 3 args

bonsaiiV · 2024-02-27T21:40:59Z

sway/commands/swap.c

+
+static void handle_container_after_move(struct sway_container *container,
+		void *data) {
+	node_set_dirty(&container->node);


redundant as containers will be set dirty in arrange_workspace

bonsaiiV · 2024-02-28T12:01:24Z

sway/commands/swap.c

+		// this needs to be more specific (focus not just every container,
+		// but single windows
+		struct sway_container *container = ws->tiling->items[0];
+		struct sway_container *first_view = container_get_first_view(container);


Why focus first view? Shouldn't workspace->focused_inactive_child be the preferred way?

bonsaiiV · 2024-03-09T21:30:05Z

I just noticed i3/i3#3952. This is what i described in my review. Since the maintainers want sway to be compatible to i3, it might be necessary to implement the enhancement for i3 first ... .

fabianscode · 2024-03-12T12:51:22Z

I just noticed i3/i3#3952. This is what i described in my review. Since the maintainers want sway to be compatible to i3, it might be necessary to implement the enhancement for i3 first ... .

the way I understood it, the intension is for sway to be a superset of i3, right?

fabianscode and others added 3 commits January 12, 2024 16:02

Add the swap_workspace command

c2eaf01

The added command allow the user to swap the contents of two workspaces. Co-authored-by: Arne <[email protected]>

Change container_get_first_view signature

88fb849

This will have the function stop the recusive calls as soon as the first view is found - unlike before

Merge branch 'master' of github.com:swaywm/sway into feature/swap-wor…

1b4d5ba

…kspace

emersion added enhancement New feature or incremental improvement window-management Configuring and arranging toplevels labels Jan 26, 2024

fabianscode added 3 commits January 28, 2024 12:12

Merge branch 'master' into feature/swap-workspace

bfd2d7e

Merge branch 'master' into feature/swap-workspace

01093f0

Merge branch 'master' into feature/swap-workspace

5c33571

fabianscode added 2 commits February 7, 2024 13:54

Merge branch 'master' into feature/swap-workspace

c0c5ad0

Merge branch 'master' into feature/swap-workspace

311d1a9

fabianscode and others added 2 commits February 26, 2024 18:33

Merge branch 'swaywm:master' into feature/swap-workspace

afc3a54

move swap_workspace logic to swap command

4cad9a1

Co-authored-by: Arne <[email protected]>

fabianscode changed the title ~~Add the swap_workspace command~~ Extend swap command to support workspaces Feb 26, 2024

bonsaiiV suggested changes Feb 28, 2024

View reviewed changes

Merge branch 'master' into feature/swap-workspace

d56dc24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend swap command to support workspaces #7907

Extend swap command to support workspaces #7907

fabianscode commented Jan 12, 2024 •

edited

Loading

bonsaiiV commented Feb 5, 2024

simonfogliato commented Feb 6, 2024

fabianscode commented Feb 7, 2024

cartok commented Feb 22, 2024

fabianscode commented Feb 26, 2024

bonsaiiV left a comment

bonsaiiV Feb 27, 2024

bonsaiiV Feb 27, 2024

bonsaiiV Feb 27, 2024

bonsaiiV Feb 27, 2024

bonsaiiV Feb 27, 2024

bonsaiiV Feb 27, 2024

bonsaiiV Feb 27, 2024

bonsaiiV Feb 28, 2024

bonsaiiV commented Mar 9, 2024 •

edited

Loading

fabianscode commented Mar 12, 2024

		@@ -344,14 +344,20 @@ set\|plus\|minus\|toggle <amount>
		"Sticks" a floating window to the current output so that it shows up on all
		workspaces.

Extend swap command to support workspaces #7907

Are you sure you want to change the base?

Extend swap command to support workspaces #7907

Conversation

fabianscode commented Jan 12, 2024 • edited Loading

bonsaiiV commented Feb 5, 2024

simonfogliato commented Feb 6, 2024

fabianscode commented Feb 7, 2024

cartok commented Feb 22, 2024

fabianscode commented Feb 26, 2024

bonsaiiV left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonsaiiV commented Mar 9, 2024 • edited Loading

fabianscode commented Mar 12, 2024

fabianscode commented Jan 12, 2024 •

edited

Loading

bonsaiiV commented Mar 9, 2024 •

edited

Loading