[PATCH v22 0/2] MR7131: DIR command improvements: Make /o by itself behave equal to how it does in Windows

[Joe Souza (＠JoeS209)]

"DIR /o" (no order specifier after /o) hasn't functioned the same as it does on Windows.  This change makes the functionality of /o by itself equivalent to behavior on Windows.

--
  v22: programs/cmd: Cleanup DIR /O logic and remove @todo_wine@ from working tests.

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

[PATCH v22 1/2] programs/cmd: Add tests patch from @epo.

[Joe Souza]

From: Joe Souza

---
 programs/cmd/tests/test_builtins.cmd     | 20 +++++++++++++
 programs/cmd/tests/test_builtins.cmd.exp | 37 ++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 8552d51930a..95d96567d25 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -2923,6 +2923,26 @@ popd
 cd
 rd /s/q foobar

+echo ------------ Testing dir /o ------------
+mkdir foobar & cd foobar
+echo AAA>a1.aa
+mkdir a1.ab
+echo A>a1.ac
+echo AA>a2.aa
+mkdir a2.ac
+echo ---
+dir /B /O:
+echo ---
+dir /B /O:GN
+echo ---
+dir /B /O:G-N
+echo ---
+dir /B /O:GNE
+echo ---
+dir /B /O:G-NE
+echo ---
+dir /B /O:G-E-N
+cd .. & rd /s/q foobar
 echo ------------ Testing attrib ------------
 rem FIXME Add tests for archive, hidden and system attributes + mixed attributes modifications
 mkdir foobar & cd foobar
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 292c24985e8..039c1a3631c 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -1754,6 +1754,43 @@ bar\baz removed
 @pwd@
 @drive@@path@foobar
 @pwd@
+------------ Testing dir /o ------------
+---
+@[e-mail]
+@[e-mail]
+@[e-mail]
+@[e-mail]
+@[e-mail]
+---
+@[e-mail]
+@[e-mail]
+a1.aa
+a1.ac
+a2.aa
+---
+a2.ac
+a1.ab
+a2.aa
+a1.ac
+a1.aa
+---
+@[e-mail]
+@[e-mail]
+@[e-mail]
+@[e-mail]
+@[e-mail]
+---
+a2.ac
+a1.ab
+@[e-mail]
+@[e-mail]
+a1.aa
+---
+a2.ac
+a1.ab
+@[e-mail]
+@[e-mail]
+a1.aa
 ------------ Testing attrib ------------
 A@spaces@@drive@@path@foobar\foo@or_broken@A       I@spaces@@drive@@path@foobar\foo
 A@spaces@@drive@@path@foobar\foo@or_broken@A       I@spaces@@drive@@path@foobar\foo

[PATCH v22 2/2] programs/cmd: Cleanup DIR /O logic and remove @todo_wine@ from working tests.

[Joe Souza]

From: Joe Souza

---
 programs/cmd/directory.c                 | 64 ++++++++++++++++++------
 programs/cmd/tests/test_builtins.cmd.exp | 32 ++++++------
 2 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/programs/cmd/directory.c b/programs/cmd/directory.c
index 3f4f7853710..3a2a5955d12 100644
--- a/programs/cmd/directory.c
+++ b/programs/cmd/directory.c
@@ -35,7 +35,8 @@ typedef enum _DISPLAYTIME

typedef enum _DISPLAYORDER
 {
-    Name = 0,
+    Unspecified = 0,
+    Name,
     Extension,
     Size,
     Date
@@ -90,16 +91,37 @@ static int __cdecl WCMD_dir_sort (const void *a, const void *b)
 {
   const WIN32_FIND_DATAW *filea = (const WIN32_FIND_DATAW *)a;
   const WIN32_FIND_DATAW *fileb = (const WIN32_FIND_DATAW *)b;
+  BOOL aDir = filea->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY;
+  BOOL bDir = fileb->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY;
   int result = 0;

-  /* If /OG or /O-G supplied, dirs go at the top or bottom, ignoring the
-     requested sort order for the directory components                   */
-  if (orderGroupDirs &&
-      ((filea->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ||
-       (fileb->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)))
-  {
-    BOOL aDir = filea->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY;
-    if (aDir) result = -1;
+  if (orderGroupDirs && dirOrder == Unspecified) {
+    /* Special case: If ordering groups and not sorting by other criteria, "." and ".." always go first. */
+    if (aDir && !lstrcmpW(filea->cFileName, L".")) {
+      result = -1;
+    } else if (bDir && !lstrcmpW(fileb->cFileName, L".")) {
+      result = 1;
+    } else if (aDir && !lstrcmpW(filea->cFileName, L"..")) {
+      result = -1;
+    } else if (bDir && !lstrcmpW(fileb->cFileName, L"..")) {
+      result = 1;
+    }
+
+    if (result) {
+      if (orderGroupDirsReverse) result = -result;
+      return result;
+    }
+  }
+
+  /* If /OG or /O-G supplied, dirs go at the top or bottom, also sorted
+     if requested sort order is by name.                                */
+  if (orderGroupDirs && (aDir || bDir)) {
+    if (aDir && bDir && dirOrder == Name) {
+      result = lstrcmpiW(filea->cFileName, fileb->cFileName);
+      if (orderReverse) result = -result;
+    } else if (aDir) {
+      result = -1;
+    }
     else result = 1;
     if (orderGroupDirsReverse) result = -result;
     return result;
@@ -674,7 +696,7 @@ RETURN_CODE WCMD_directory(WCHAR *args)
   orderByCol = FALSE;
   separator  = TRUE;
   dirTime = Written;
-  dirOrder = Name;
+  dirOrder = Unspecified;
   orderReverse = FALSE;
   orderGroupDirs = FALSE;
   orderGroupDirsReverse = FALSE;
@@ -744,13 +766,16 @@ RETURN_CODE WCMD_directory(WCHAR *args)
               if (*p==':') p++;  /* Skip optional : */
               while (*p && *p != '/') {
                 WINE_TRACE("Processing subparm '%c' (in %s)\n", *p, wine_dbgstr_w(quals));
+                /* Options N,E,S,D are mutually-exclusive, first encountered takes precedence. */
                 switch (*p) {
-                case 'N': dirOrder = Name;       break;
-                case 'E': dirOrder = Extension;  break;
-                case 'S': dirOrder = Size;       break;
-                case 'D': dirOrder = Date;       break;
-                case '-': if (*(p+1)=='G') orderGroupDirsReverse=TRUE;
-                          else orderReverse = TRUE;
+                case 'N': if (dirOrder == Unspecified) dirOrder = Name;       break;
+                case 'E': if (dirOrder == Unspecified) dirOrder = Extension;  break;
+                case 'S': if (dirOrder == Unspecified) dirOrder = Size;       break;
+                case 'D': if (dirOrder == Unspecified) dirOrder = Date;       break;
+                case '-': if (dirOrder == Unspecified) {
+                            if (*(p+1)=='G') orderGroupDirsReverse=TRUE;
+                            else if (*(p+1)=='N'||*(p+1)=='E'||*(p+1)=='S'||*(p+1)=='D') orderReverse = TRUE;
+                          }
                           break;
                 case 'G': orderGroupDirs = TRUE; break;
                 default:
@@ -760,6 +785,13 @@ RETURN_CODE WCMD_directory(WCHAR *args)
                 }
                 p++;
               }
+              /* Handle default case of /O specified by itself, with no specific options.
+                 This is equivalent to /O:GN. */
+              if (dirOrder == Unspecified && !orderGroupDirs) {
+                orderGroupDirs = TRUE;
+                orderGroupDirsReverse = FALSE;
+                dirOrder = Name;
+              }
               p = p - 1; /* So when step on, move to '/' */
               break;
     case 'A': p = p + 1;
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 039c1a3631c..19a6ec78c10 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -1756,14 +1756,14 @@ bar\baz removed
 @pwd@
 ------------ Testing dir /o ------------
 ---
-@[e-mail]
-@[e-mail]
-@[e-mail]
-@[e-mail]
-@[e-mail]
+a1.ab
+a2.ac
+a1.aa
+a1.ac
+a2.aa
 ---
-@[e-mail]
-@[e-mail]
+a1.ab
+a2.ac
 a1.aa
 a1.ac
 a2.aa
@@ -1774,22 +1774,22 @@ a2.aa
 a1.ac
 a1.aa
 ---
-@[e-mail]
-@[e-mail]
-@[e-mail]
-@[e-mail]
-@[e-mail]
+a1.ab
+a2.ac
+a1.aa
+a1.ac
+a2.aa
 ---
 a2.ac
 a1.ab
-@[e-mail]
-@[e-mail]
+a2.aa
+a1.ac
 a1.aa
 ---
 a2.ac
 a1.ab
-@[e-mail]
-@[e-mail]
+a1.ac
+a2.aa
 a1.aa
 ------------ Testing attrib ------------
 A@spaces@@drive@@path@foobar\foo@or_broken@A       I@spaces@@drive@@path@foobar\foo

[PATCH v22 2/2] programs/cmd: Cleanup DIR /O logic and remove @todo_wine@ from working tests.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=151253

Your paranoid android.

=== debian11b (64 bit WoW report) ===

mf:
mf.c:6785: Test failed: Unexpected hr 0.

user32:
input.c:4306: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000007800FE, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032

[Joe Souza (＠JoeS209)]

I was able to squash the two commits into one.  Now the merge request has the two commits as you suggested.  The first commit contains the test changes from your patch, and the second commit contains my code changes and the removal of the @todo_wine@ from the tests that are passing.

[eric pouech (＠epo)]

thanks for updating

globally looks good

last nitpick: the title of first patch should be more meaningful, like "Add tests for command DIR /O."