[PATCH 0/2] MR11554: cmd: Canonicalize the echo of redirections and separators.

[Lokesh Poovaragan (＠lokeshpoovaragan)]

When echo:ing a command, match native behavior by only showing the last redirection on a given stream, and by adding spaces around redirections and separators.

When a command has several redirections on the same stream, native only echoes the last one.

Signed-off-by: Lokesh Poovaragan [e-mail]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11554

[PATCH 1/2] cmd/tests: Add tests for echo of multiple redirections.

[Lokesh Poovaragan]

From: Lokesh Poovaragan <[e-mail]>

When a command has several redirections on the same stream,
native only echoes the last one.

Signed-off-by: Lokesh Poovaragan <[e-mail]>
---
 programs/cmd/tests/test_builtins.cmd     | 1 +
 programs/cmd/tests/test_builtins.cmd.exp | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 34d1f076485..a5b31566a45 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -32,6 +32,7 @@ echo  @tab@word
 echo@tab@@tab@word
 echo @tab@ on @space@
> nul echo a
> +> x > y echo a
>  if@tab@1    ==           2 then @echo a
>  @rem native stores the keyword (and preserve the case) :-(
>  IF@tab@1    ==           2 ThEn @EchO a
> diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
> index dc6420e1432..b3cd2a687f6 100644
> --- a/programs/cmd/tests/test_builtins.cmd.exp
> +++ b/programs/cmd/tests/test_builtins.cmd.exp
> @@ -91,6 +91,8 @@ word

@todo_wine@@pwd@>echo a 1>nul@space@
 
+@todo_wine@@pwd@>echo a 1>y@space@
+
 @pwd@>if 1 == 2 then @echo a@space@
 
 @todo_wine@@pwd@>IF 1 == 2 ThEn @EchO a@space@
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/11554

[PATCH 2/2] cmd: Canonicalize the echo of redirections and separators.

[Lokesh Poovaragan]

From: Lokesh Poovaragan <[e-mail]>

When echo:ing a command, match native behavior by only showing
the last redirection on a given stream, and by adding spaces
around redirections and separators. The todo_wine markers in
the tests are removed as the fixed output now matches native.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=21227

Signed-off-by: Lokesh Poovaragan <[e-mail]>
---
 programs/cmd/tests/test_builtins.cmd.exp |  8 ++---
 programs/cmd/wcmdmain.c                  | 44 +++++++++++++++++++-----
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index b3cd2a687f6..feff50f6c1d 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -89,9 +89,9 @@ word
 
 @pwd@>echo @tab@ on @space@@space@
 
-@todo_wine@@pwd@>echo a 1>nul@space@
+@pwd@>echo a 1>nul@space@
 
-@todo_wine@@pwd@>echo a 1>y@space@
+@pwd@>echo a 1>y@space@
 
 @pwd@>if 1 == 2 then @echo a@space@
 
@@ -106,14 +106,14 @@ word
 AA@\x08@BB
 --- @ with chains and brackets
 
-@todo_wine@@pwd@>(echo the @ character chains until  && )  && echo and can hide brackets  || ()  ||@space@
+@pwd@>(echo the @ character chains until  && )  && echo and can hide brackets  || ()  ||@space@
 the @ character chains until
 we leave the current depth
 and can hide brackets
 @todo_wine@---
 foo1
 
-@todo_wine@@pwd@>echo a  1>nul  &&@space@
+@pwd@>echo a  1>nul  &&@space@
 foo2
 ---
 noecho1
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index 467209a053b..d938df31492 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -3331,16 +3331,23 @@ static BOOL rebuild_append_redirection(struct command_rebuild *rb, const CMD_RED
         rebuild_expand_and_append(rb, dst, expand);
 }
 
-static BOOL rebuild_append_all_redirections(struct command_rebuild *rb, const CMD_NODE *node, BOOL expand)
+static BOOL rebuild_append_all_redirections(struct command_rebuild *rb, const CMD_NODE *node, BOOL expand, BOOL in_echo)
 {
     CMD_REDIRECTION *redir;
     BOOL ret = TRUE;
 
     for (redir = node->redirects; ret && redir != NULL; redir = redir->next)
     {
+        CMD_REDIRECTION *next;
+
+        /* if we have several elements changing same std stream, only use last one */
+        for (next = redir->next; next; next = next->next)
+            if (redir->fd == next->fd) break;
+        if (next) continue;
         if (rb->pos && !iswspace(rb->buffer[rb->pos - 1]))
             ret = ret && rebuild_append(rb, L" ");
         ret = ret && rebuild_append_redirection(rb, redir, expand);
+        if (in_echo) ret = ret && rebuild_append(rb, L" ");
     }
     return ret;
 }
@@ -3377,11 +3384,21 @@ static BOOL rebuild_command_binary(struct command_rebuild *rb, const CMD_NODE *n
     default: return FALSE;
     }
 
-    ret = rebuild_append_command(rb, node->left, new_rbflags) &&
-        ((node->left->op == CMD_SINGLE && node->op == CMD_CONCAT) ? rebuild_append(rb, L" ") : TRUE);
+    ret = rebuild_append_command(rb, node->left, new_rbflags);
     if (!rbflags.in_echo || rebuild_shall_echo(node->left))
     {
-        ret = ret && rebuild_append(rb, op_string);
+        if (rbflags.in_echo)
+        {
+            /* native adds an extra space before and after the operator */
+            ret = ret && rebuild_append(rb, L" ") &&
+                rebuild_append(rb, op_string) &&
+                rebuild_append(rb, L" ");
+        }
+        else
+        {
+            ret = ret && ((node->left->op == CMD_SINGLE && node->op == CMD_CONCAT) ? rebuild_append(rb, L" ") : TRUE) &&
+                rebuild_append(rb, op_string);
+        }
         ret = ret && rebuild_append_command(rb, node->right, new_rbflags);
     }
     return ret;
@@ -3557,16 +3574,27 @@ static BOOL rebuild_append_command(struct command_rebuild *rb, const CMD_NODE *n
     case CMD_BLOCK:
         {
             struct rebuild_flags new_rbflags = {.depth = rbflags.depth = 1, .in_echo = rbflags.in_echo};
-            ret = rebuild_append(rb, L"( ") &&
-                rebuild_append_command(rb, node->block, new_rbflags) &&
-                rebuild_append(rb, L" ) ");
+            if (rbflags.in_echo)
+            {
+                /* native glues the block content to the parentheses */
+                ret = rebuild_append(rb, L"(") &&
+                    rebuild_append_command(rb, node->block, new_rbflags) &&
+                    rebuild_append(rb, L")") &&
+                    rebuild_append(rb, L" ");
+            }
+            else
+            {
+                ret = rebuild_append(rb, L"( ") &&
+                    rebuild_append_command(rb, node->block, new_rbflags) &&
+                    rebuild_append(rb, L" ) ");
+            }
         }
         break;
     default:
         FIXME("Shouldn't happen\n");
         ret = FALSE;
     }
-    ret = ret && rebuild_append_all_redirections(rb, node, rbflags.depth == 0);
+    ret = ret && rebuild_append_all_redirections(rb, node, rbflags.depth == 0, rbflags.in_echo);
 
     return ret;
 }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/11554