This series of patches is heavily inspired by https://gitlab.winehq.org/wine/wine/-/merge_requests/277 and solves problems related to - whitespace only lines in blocks - EOF in the middle of the blocks - lines with caret followed by EOF
From: Alexander Merkle alexander.merkle@lauterbach.com
Heavily inspired by https://gitlab.winehq.org/wine/wine/-/merge_requests/277 https://gitlab.winehq.org/bernhardu/wine/-/tree/51599_cmd_brackets by Bernhard Übelacker bernhardu@mailbox.org . Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51599
Signed-off-by: Alexander Merkle alexander.merkle@lauterbach.com --- programs/cmd/tests/test_builtins.cmd | 14 ++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 3 +++ programs/cmd/wcmdmain.c | 18 +++++++++++++++++- 3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 510a1ba5931..35caf136e4c 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1029,6 +1029,20 @@ if not exist %windir% ( ) else ( echo windir does exist ) +if 1 == 0 ( + echo 1 == 0 should not be true +@space@@tab@ +) else echo block containing a line with just spaces seems to work +if 1 == 0 ( +@space@@tab@ + echo 1 == 0 should not be true +) else echo block containing a line with just spaces seems to work +if 1 == 0 ( +@space@@tab@ + echo 1 == 0 should not be true +@tab@ +@space@ +) else echo block containing a line with just spaces seems to work echo --- case sensitivity with and without /i option if bar==BAR echo if does not default to case sensitivity if not bar==BAR echo if seems to default to case sensitivity diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 0f48a823109..379225f38e5 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -707,6 +707,9 @@ comparison operators surrounded by brackets seem to work comparison operators surrounded by brackets seem to work windir is defined windir does exist +block containing a line with just spaces seems to work +block containing a line with just spaces seems to work +block containing a line with just spaces seems to work --- case sensitivity with and without /i option if seems to default to case sensitivity if /i seems to work diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 843fef8ea50..3759b1605da 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -1804,6 +1804,22 @@ static BOOL WCMD_IsEndQuote(const WCHAR *quote, int quoteIndex) return FALSE; }
+/*************************************************************************** + * WCMD_isEmptyOrJustWhiteSpace + * + * Returns TRUE if str is empty or contains just whitespace characters, + * otherwise returns FALSE. + */ +static BOOL WCMD_isEmptyOrJustWhiteSpace(const WCHAR *str) +{ + while (*str != '\0') { + if (*str != ' ' && *str != '\t') + return FALSE; + str++; + } + return TRUE; +} + /*************************************************************************** * WCMD_ReadAndParseLine * @@ -2326,7 +2342,7 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE } else break; }
- } while (*extraData == 0x00); + } while (WCMD_isEmptyOrJustWhiteSpace(extraData)); curPos = extraSpace;
/* Skip preceding whitespace */
From: Alexander Merkle alexander.merkle@lauterbach.com
as discussed in https://gitlab.winehq.org/wine/wine/-/merge_requests/277
Signed-off-by: Alexander Merkle alexander.merkle@lauterbach.com --- programs/cmd/tests/rsrc.rc | 12 ++++++++++++ programs/cmd/tests/test_block_if_eof1.cmd | 5 +++++ programs/cmd/tests/test_block_if_eof1.cmd.exp | 1 + programs/cmd/tests/test_block_if_eof2.cmd | 5 +++++ programs/cmd/tests/test_block_if_eof2.cmd.exp | 1 + programs/cmd/wcmdmain.c | 6 ++++-- 6 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 programs/cmd/tests/test_block_if_eof1.cmd create mode 100644 programs/cmd/tests/test_block_if_eof1.cmd.exp create mode 100644 programs/cmd/tests/test_block_if_eof2.cmd create mode 100644 programs/cmd/tests/test_block_if_eof2.cmd.exp
diff --git a/programs/cmd/tests/rsrc.rc b/programs/cmd/tests/rsrc.rc index affe04c2bdd..ca45543cfa4 100644 --- a/programs/cmd/tests/rsrc.rc +++ b/programs/cmd/tests/rsrc.rc @@ -22,6 +22,18 @@ test_builtins.cmd TESTCMD "test_builtins.cmd" /* @makedep: test_builtins.cmd.exp */ test_builtins.cmd.exp TESTOUT "test_builtins.cmd.exp"
+/* @makedep: test_block_if_eof1.cmd */ +test_block_if_eof1.cmd TESTCMD "test_block_if_eof1.cmd" + +/* @makedep: test_block_if_eof1.cmd.exp */ +test_block_if_eof1.cmd.exp TESTOUT "test_block_if_eof1.cmd.exp" + +/* @makedep: test_block_if_eof2.cmd */ +test_block_if_eof2.cmd TESTCMD "test_block_if_eof2.cmd" + +/* @makedep: test_block_if_eof2.cmd.exp */ +test_block_if_eof2.cmd.exp TESTOUT "test_block_if_eof2.cmd.exp" + /* @makedep: test_cmdline.cmd */ test_cmdline.cmd TESTCMD "test_cmdline.cmd"
diff --git a/programs/cmd/tests/test_block_if_eof1.cmd b/programs/cmd/tests/test_block_if_eof1.cmd new file mode 100644 index 00000000000..9b2ee2e97f3 --- /dev/null +++ b/programs/cmd/tests/test_block_if_eof1.cmd @@ -0,0 +1,5 @@ +@echo off +echo Test if with unbalanced parentheses + +if 1 == 1 ( + echo should not be executed diff --git a/programs/cmd/tests/test_block_if_eof1.cmd.exp b/programs/cmd/tests/test_block_if_eof1.cmd.exp new file mode 100644 index 00000000000..419b130e442 --- /dev/null +++ b/programs/cmd/tests/test_block_if_eof1.cmd.exp @@ -0,0 +1 @@ +Test if with unbalanced parentheses diff --git a/programs/cmd/tests/test_block_if_eof2.cmd b/programs/cmd/tests/test_block_if_eof2.cmd new file mode 100644 index 00000000000..2c34647a732 --- /dev/null +++ b/programs/cmd/tests/test_block_if_eof2.cmd @@ -0,0 +1,5 @@ +@echo off +echo Test if with unbalanced parentheses + +if 1 == 0 ( + echo should not be executed diff --git a/programs/cmd/tests/test_block_if_eof2.cmd.exp b/programs/cmd/tests/test_block_if_eof2.cmd.exp new file mode 100644 index 00000000000..419b130e442 --- /dev/null +++ b/programs/cmd/tests/test_block_if_eof2.cmd.exp @@ -0,0 +1 @@ +Test if with unbalanced parentheses diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 3759b1605da..453da83aa2a 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2328,8 +2328,10 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE do { WINE_TRACE("Read more input\n"); if (!context) WCMD_output_asis( WCMD_LoadMessage(WCMD_MOREPROMPT)); - if (!WCMD_fgets(extraData, MAXSTRING, readFrom)) - break; + if (!WCMD_fgets(extraData, MAXSTRING, readFrom)) { + /* EOF in parentheses - abort don't execute block */ + return NULL; + }
/* Edge case for carets - a completely blank line (i.e. was just CRLF) is oddly added as an LF but then more data is received (but
From: Alexander Merkle alexander.merkle@lauterbach.com
--- programs/cmd/tests/rsrc.rc | 6 ++++++ programs/cmd/tests/test_caret_eof.cmd | 3 +++ programs/cmd/tests/test_caret_eof.cmd.exp | 3 +++ programs/cmd/wcmdmain.c | 6 ++++++ 4 files changed, 18 insertions(+) create mode 100644 programs/cmd/tests/test_caret_eof.cmd create mode 100644 programs/cmd/tests/test_caret_eof.cmd.exp
diff --git a/programs/cmd/tests/rsrc.rc b/programs/cmd/tests/rsrc.rc index ca45543cfa4..382e297c468 100644 --- a/programs/cmd/tests/rsrc.rc +++ b/programs/cmd/tests/rsrc.rc @@ -34,6 +34,12 @@ test_block_if_eof2.cmd TESTCMD "test_block_if_eof2.cmd" /* @makedep: test_block_if_eof2.cmd.exp */ test_block_if_eof2.cmd.exp TESTOUT "test_block_if_eof2.cmd.exp"
+/* @makedep: test_caret_eof.cmd */ +test_caret_eof.cmd TESTCMD "test_caret_eof.cmd" + +/* @makedep: test_caret_eof.cmd.exp */ +test_caret_eof.cmd.exp TESTOUT "test_caret_eof.cmd.exp" + /* @makedep: test_cmdline.cmd */ test_cmdline.cmd TESTCMD "test_cmdline.cmd"
diff --git a/programs/cmd/tests/test_caret_eof.cmd b/programs/cmd/tests/test_caret_eof.cmd new file mode 100644 index 00000000000..d0157171925 --- /dev/null +++ b/programs/cmd/tests/test_caret_eof.cmd @@ -0,0 +1,3 @@ +@echo off +echo Test caret followed by EOF +echo foo^ diff --git a/programs/cmd/tests/test_caret_eof.cmd.exp b/programs/cmd/tests/test_caret_eof.cmd.exp new file mode 100644 index 00000000000..9c99c033e6b --- /dev/null +++ b/programs/cmd/tests/test_caret_eof.cmd.exp @@ -0,0 +1,3 @@ +Test caret followed by EOF +foo + diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 453da83aa2a..2a12014fcde 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2330,6 +2330,12 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE if (!context) WCMD_output_asis( WCMD_LoadMessage(WCMD_MOREPROMPT)); if (!WCMD_fgets(extraData, MAXSTRING, readFrom)) { /* EOF in parentheses - abort don't execute block */ + /* caret followed by EOF - execute line */ + if (lastWasCaret && (curDepth == 0) && (*extraSpace == 0x00)) { + *extraData++ = '\r'; + *extraData = 0x00; + break; + } return NULL; }
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=139024
Your paranoid android.
=== w7u_2qxl (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w7u_adm (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w7u_el (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w8 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w8adm (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w864 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064v1507 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064v1809 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064_tsign (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w10pro64 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w10pro64_en_AE_u8 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w11pro64 (32 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w7pro64 (64 bit report) ===
cmd.exe: batch.c:89: Test failed: CreateFile failed batch.c:89: Test failed: CreateFile failed batch.c:89: Test failed: CreateFile failed batch.c:89: Test failed: CreateFile failed
=== w864 (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064v1507 (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064v1809 (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064_2qxl (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064_adm (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w1064_tsign (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w10pro64 (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w10pro64_ar (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w10pro64_ja (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w10pro64_zh_CN (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
=== w11pro64_amd (64 bit report) ===
cmd.exe: batch.c:347: Test failed: unexpected end of output in line 2, missing
eric pouech (@epo) commented about programs/cmd/tests/test_builtins.cmd.exp:
comparison operators surrounded by brackets seem to work windir is defined windir does exist +block containing a line with just spaces seems to work +block containing a line with just spaces seems to work +block containing a line with just spaces seems to work --- case sensitivity with and without /i option
there's something wrong here the the "echo --- case..." command gets lots in the output
eric pouech (@epo) commented about programs/cmd/wcmdmain.c:
} else break; }
} while (*extraData == 0x00);
what about using WCMD_skip_leading_spaces() instead?
eric pouech (@epo) commented about programs/cmd/tests/rsrc.rc:
/* @makedep: test_builtins.cmd.exp */ test_builtins.cmd.exp TESTOUT "test_builtins.cmd.exp"
+/* @makedep: test_block_if_eof1.cmd */
adding separated test cases looks a bit overkill could you do create the test files from withing the existing .cmd file (search for tmp.cmd for examples)
eric pouech (@epo) commented about programs/cmd/wcmdmain.c:
if (!context) WCMD_output_asis( WCMD_LoadMessage(WCMD_MOREPROMPT)); if (!WCMD_fgets(extraData, MAXSTRING, readFrom)) { /* EOF in parentheses - abort don't execute block */
/* caret followed by EOF - execute line */
I wonder if this part is actually covered by your tests (not sure it should as it requires to have the exact right length before) so I wonder if native cmd has the same behavior
On Tue Oct 24 17:51:17 2023 +0000, eric pouech wrote:
what about using WCMD_skip_leading_spaces() instead?
Doable, would result in ``` } while (!*WCMD_skip_leading_spaces(extraData)); ``` I will make this part of the next push, I can integrate(fixup) this in the final force-push.
On Tue Oct 24 17:51:15 2023 +0000, eric pouech wrote:
there's something wrong here the the "echo --- case..." command gets lots in the output
Sorry, I didn't get that. Is there a fail in the CI / tests that I missed?
On Tue Oct 24 17:51:19 2023 +0000, eric pouech wrote:
I wonder if this part is actually covered by your tests (not sure it should as it requires to have the exact right length before) so I wonder if native cmd has the same behavior
That was the purpose of `test_caret_eof` - I think this is a very rare cornercase - I just added is for completeness. Or the meaning of `caret` something different than the line continuation marker `^` here?
-
Alexander Merkle -
eric pouech (＠epo)
-
Marvin -
userid0x0 (＠userid0x0)