From 854616ed2ece299317691406ff44b6370bb3fa98 Mon Sep 17 00:00:00 2001
From: Orestis Floros <orestisflo@gmail.com>
Date: Sat, 27 Nov 2021 22:48:48 +0100
Subject: [PATCH] user_output_names_find_next: Always initialize target_output

This way, if the user has provided a valid, existing output in the list
of outputs, the focus & move workspace to output commands will not
report a misleading failure.

Side-effect is that the command code will try to execute a no-op e.g. by
moving the workspace to the output it already is on. But that's what the
user is actually requesting in this case and it shouldn't be a problem.

Fixes #4691
---
 release-notes/bugfixes/4-failed-workspace-output |  1 +
 src/commands.c                                   | 16 ++++++++--------
 .../t/543-move-workspace-to-multiple-outputs.t   | 10 ++++++++++
 3 files changed, 19 insertions(+), 8 deletions(-)
 create mode 100644 release-notes/bugfixes/4-failed-workspace-output

diff --git a/release-notes/bugfixes/4-failed-workspace-output b/release-notes/bugfixes/4-failed-workspace-output
new file mode 100644
index 00000000..b3d23957
--- /dev/null
+++ b/release-notes/bugfixes/4-failed-workspace-output
@@ -0,0 +1 @@
+fix wrong failed reply on move workspace to output
diff --git a/src/commands.c b/src/commands.c
index ac02ba6e..a4af1a39 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -1052,6 +1052,14 @@ static Output *user_output_names_find_next(user_output_names_head *names, Output
     Output *target_output = NULL;
     user_output_name *uo;
     TAILQ_FOREACH (uo, names, user_output_names) {
+        if (!target_output) {
+            /* The first available output from the list is used in 2 cases:
+             * 1. When we must wrap around the user list. For example, if user
+             * specifies outputs A B C and C is `current_output`.
+             * 2. When the current output is not in the user list. For example,
+             * user specifies A B C and D is `current_output`. */
+            target_output = get_output_from_string(current_output, uo->name);
+        }
         if (strcasecmp(output_primary_name(current_output), uo->name) == 0) {
             /* The current output is in the user list */
             while (true) {
@@ -1071,14 +1079,6 @@ static Output *user_output_names_find_next(user_output_names_head *names, Output
             }
             break;
         }
-        if (!target_output) {
-            /* The first available output from the list is used in 2 cases:
-             * 1. When we must wrap around the user list. For example, if user
-             * specifies outputs A B C and C is `current_output`.
-             * 2. When the current output is not in the user list. For example,
-             * user specifies A B C and D is `current_output`. */
-            target_output = get_output_from_string(current_output, uo->name);
-        }
     }
     return target_output;
 }
diff --git a/testcases/t/543-move-workspace-to-multiple-outputs.t b/testcases/t/543-move-workspace-to-multiple-outputs.t
index 72614fe5..c2826ee5 100644
--- a/testcases/t/543-move-workspace-to-multiple-outputs.t
+++ b/testcases/t/543-move-workspace-to-multiple-outputs.t
@@ -39,6 +39,16 @@ sub is_ws {
     is(get_output_for_workspace("$ws_num"), "fake-$out_num", "Workspace $ws_num -> $out_num: $msg");
 }
 
+###############################################################################
+# Test moving workspace to same output
+# See issue #4691
+###############################################################################
+is_ws(1, 0, 'sanity check');
+
+my $reply = cmd '[con_mark=aa] move workspace to output fake-0';
+is_ws(1, 0, 'workspace did not move');
+ok($reply->[0]->{success}, 'reply success');
+
 ###############################################################################
 # Test using "next" special keyword
 ###############################################################################