[PATCH v3 0/3] MR4168: cmd: Handle caret / blocks with whitespace only lines better
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 -- v3: cmd: Handle lines terminated with caret followed by EOF cmd: Handle unterminated/unbalanced parentheses cmd: Handle whitespace only lines in parentheses blocks https://gitlab.winehq.org/wine/wine/-/merge_requests/4168
From: Alexander Merkle <alexander.merkle(a)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(a)mailbox.org> . Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51599 Signed-off-by: Alexander Merkle <alexander.merkle(a)lauterbach.com> --- programs/cmd/tests/test_builtins.cmd | 14 ++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 3 +++ programs/cmd/wcmdmain.c | 7 +++++-- 3 files changed, 22 insertions(+), 2 deletions(-) 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 +(a)space@@tab@ +) else echo block containing a line with just spaces seems to work +if 1 == 0 ( +(a)space@@tab@ + echo 1 == 0 should not be true +) else echo block containing a line with just spaces seems to work +if 1 == 0 ( +(a)space@@tab@ + echo 1 == 0 should not be true +(a)tab@ +(a)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..5d5c18eacb6 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2326,11 +2326,14 @@ WCHAR *WCMD_ReadAndParseLine(const WCHAR *optionalcmd, CMD_LIST **output, HANDLE } else break; } - } while (*extraData == 0x00); + /* Skip preceding whitespace + - continue loop if line is empty/whitespace only */ + extraData = WCMD_skip_leading_spaces(extraData); + } while (*extraData == 0x0); curPos = extraSpace; /* Skip preceding whitespace */ - while (*curPos == ' ' || *curPos == '\t') curPos++; + curPos = WCMD_skip_leading_spaces(curPos); /* Replace env vars if in a batch context */ if (context) handleExpansion(curPos, FALSE, FALSE); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4168
From: Alexander Merkle <alexander.merkle(a)lauterbach.com> as discussed in https://gitlab.winehq.org/wine/wine/-/merge_requests/277 Signed-off-by: Alexander Merkle <alexander.merkle(a)lauterbach.com> --- programs/cmd/tests/test_builtins.cmd | 13 +++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 3 +++ programs/cmd/wcmdmain.c | 6 ++++-- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 35caf136e4c..d72fcf2385a 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -1043,6 +1043,19 @@ if 1 == 0 ( @tab@ @space@ ) else echo block containing a line with just spaces seems to work +echo --- if with unbalanced parentheses +echo @echo off>block_if_eof1.cmd +echo echo sanity1>>block_if_eof1.cmd +echo if 1 == 1 (>>block_if_eof1.cmd +echo echo should not be executed>>block_if_eof1.cmd +call block_if_eof1.cmd +del block_if_eof1.cmd +echo @echo off>block_if_eof2.cmd +echo echo sanity2>>block_if_eof2.cmd +echo if 1 == 0 (>>block_if_eof2.cmd +echo echo should not be executed>>block_if_eof2.cmd +call block_if_eof2.cmd +del block_if_eof2.cmd 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 379225f38e5..145f97a550a 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -710,6 +710,9 @@ 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 +--- if with unbalanced parentheses +sanity1 +sanity2 --- 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 5d5c18eacb6..473ead28ab9 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2312,8 +2312,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 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4168
From: Alexander Merkle <alexander.merkle(a)lauterbach.com> Signed-off-by: Alexander Merkle <alexander.merkle(a)lauterbach.com> --- programs/cmd/tests/test_builtins.cmd | 6 ++++++ programs/cmd/tests/test_builtins.cmd.exp | 3 +++ programs/cmd/wcmdmain.c | 6 ++++++ 3 files changed, 15 insertions(+) diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index d72fcf2385a..38423e3a72a 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -262,6 +262,12 @@ echo %WINE_FOO% echo %ErrorLevel% set WINE_FOO= +echo --- circumflex EOF behaviour +echo @echo off>caret_eof.cmd +echo echo foo^^>>caret_eof.cmd +call caret_eof.cmd +del caret_eof.cmd + echo ------------ Testing chains ------------ rem The chain operators have the following bottom-up precedence: rem 'else' precedes nothing and matches the closest unmatched 'if' in the same bracket depth diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 145f97a550a..d265d5c4ed4 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -255,6 +255,9 @@ WINE_FOO=bar | baz WINE_FOO=bar ^| baz bar | baz 0 +--- circumflex EOF behaviour +foo + ------------ Testing chains ------------ --- chain success a1 diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 473ead28ab9..d901d85250f 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -2314,6 +2314,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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4168
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=139116 Your paranoid android. === w7u_2qxl (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w7u_adm (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w7u_el (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w8 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w8adm (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w864 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064v1507 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064v1809 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064_tsign (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w10pro64 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w10pro64_en_AE_u8 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w11pro64 (32 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w7pro64 (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w864 (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064v1507 (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064v1809 (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064_2qxl (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064_adm (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w1064_tsign (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w10pro64 (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w10pro64_ar (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w10pro64_ja (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w10pro64_zh_CN (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2 === w11pro64_amd (64 bit report) === cmd.exe: batch.c:313: Test failed: excess characters on line 260 (got '------------ Testing chains ------------', wanted '') batch.c:347: Test failed: unexpected end of output in line 717, missing sanity2
On Wed Oct 25 20:40:10 2023 +0000, eric pouech wrote:
sorry, I've been confused by gitlab which turns in red some part (which means line removed), but it's done wrongly (likely gitlab tool for setting colors uses some (too) simple regexp:s) tests written like this are ok Yeah, now I see it as well. The coloring seems to be a pitfall. I think we can close this subthread.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4168#note_49832
On Wed Oct 25 20:35:41 2023 +0000, eric pouech wrote:
it seems to me that the handling of the EOF could be written more simply (don't have precise idea how) Feel free to drop the last commit. I can create a separate pull-request as well. For me the whitespace problem has much higher priority.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4168#note_49833
On Wed Oct 25 20:22:13 2023 +0000, eric pouech wrote:
I'd like better (which I believe is how it's done in other places) ``` extraData = WCMD_skip_leading_spaces(extraData); } while (*extraData == 0x00); ``` done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4168#note_49834
I rebased my work into three commits: - whitespace only lines - parentheses blocks that end with EOF/are unbalanced - the caret/circumflex behavior if the next line is missing . I only worked on the caret/circumflex behavior as I worked on the same code location. Feel free to drop the caret/circumflex work. The threads above got out-of-sync as I force pushed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4168#note_49835
participants (3)
-
Alexander Merkle -
Marvin -
userid0x0 (＠userid0x0)