code review, restored backwards compatibility of the simplest escape of quote-chars (so reverted several tests winpipe-7.x)

2 files changed, 29 insertions, 16 deletions

diff --git a/tests/winPipe.test b/tests/winPipe.test
index c375d8f..5c6eac8 100644
--- a/tests/winPipe.test
+++ b/tests/winPipe.test
@@ -362,22 +362,22 @@ test winpipe-7.2 {BuildCommandLine: null arguments} {win exec} {
 } {foo "" bar}
 test winpipe-7.3 {BuildCommandLine: dbl quote quoting #1} {win exec} {
     exec $env(COMSPEC) /c echo foo "\"" bar
-} {foo "\"" bar}
+} {foo \" bar}
 test winpipe-7.4 {BuildCommandLine: dbl quote quoting #2} {win exec} {
     exec $env(COMSPEC) /c echo foo {""} bar
-} {foo "\"\"" bar}
+} {foo \"\" bar}
 test winpipe-7.5 {BuildCommandLine: dbl quote quoting #3} {win exec} {
     exec $env(COMSPEC) /c echo foo "\" " bar
 } {foo "\" " bar}
 test winpipe-7.6 {BuildCommandLine: dbl quote quoting #4} {win exec} {
     exec $env(COMSPEC) /c echo foo {a="b"} bar
-} {foo "a=\"b\"" bar}
+} {foo a=\"b\" bar}
 test winpipe-7.7 {BuildCommandLine: dbl quote quoting #5} {win exec} {
     exec $env(COMSPEC) /c echo foo {a = "b"} bar
 } {foo "a = \"b\"" bar}
 test winpipe-7.8 {BuildCommandLine: dbl quote quoting #6} {win exec} {
     exec $env(COMSPEC) /c echo {"hello"} {""hello""} {"""hello"""} {"\"hello\""} {he llo} "he \" llo"
-} {"\"hello\"" "\"\"hello\"\"" "\"\"\"hello\"\"\"" "\"\\\"hello\\\"\"" "he llo" "he \" llo"}
+} {\"hello\" \"\"hello\"\" \"\"\"hello\"\"\" \"\\\"hello\\\"\" "he llo" "he \" llo"}
 test winpipe-7.9 {BuildCommandLine: N backslashes followed a quote rule #1} {win exec} {
     exec $env(COMSPEC) /c echo foo \\ bar
 } {foo \ bar}
diff --git a/win/tclWinPipe.c b/win/tclWinPipe.c
index 728e43a..21bdcec 100644
--- a/win/tclWinPipe.c
+++ b/win/tclWinPipe.c
@@ -1647,21 +1647,27 @@ BuildCommandLine(
 	/* Quote flags:
 	 *   1 - escape argument;
 	 *   2 - previous arguments chain contains unpaired quote-char;
+	 *   4 - enclose in quotes;
 	 */
-	quote &= ~1; /* reset escape flag */
+	quote &= ~5; /* reset escape flags */
 	bspos = NULL;
 	if (arg[0] == '\0') {
-	    quote = 1;
+	    quote = 5;
 	} else {
 	    int count;
 	    Tcl_UniChar ch;
 	    for (start = arg; *start != '\0'; start += count) {
 		count = Tcl_UtfToUniChar(start, &ch);
-		if (Tcl_UniCharIsSpace(ch) ||
-		   (count == 1 && (*start=='"' || strchr(specMetaChars, *start)))
-		) {
-		    quote |= 1; /* set escape flag - must be quoted */
-		    break;
+		if (count == 1) {
+		    if (Tcl_UniCharIsSpace(ch) ||
+			strchr(specMetaChars, *start)
+		    ) {
+			quote |= 5; /* set escape flag & must be quoted */
+			break;
+		    }
+		    if (*start == '"') {
+			quote |= 1; /* set escape flag */
+		    }
 		}
 	    }
 	}
@@ -1670,7 +1676,9 @@ BuildCommandLine(
 	    Tcl_DStringAppend(&ds, arg, -1);
 	} else {
 	    /* start of argument (main opening quote-char) */
-	    Tcl_DStringAppend(&ds, "\"", 1);
+	    if (quote & 4) {
+		Tcl_DStringAppend(&ds, "\"", 1);
+	    }
 	    start = arg;
 	    for (special = arg; *special != '\0'; ) {
 		/* position of `\` is important before quote or at end (equal `\"` because quoted) */
@@ -1708,10 +1716,15 @@ BuildCommandLine(
 		bspos = NULL; /* reset last backslash possition (not interesting) */
 		special++;
 	    }
-	    /* rest of argument (and escape backslashes before closing main quote) */
-	    QuoteCmdLineBackslash(&ds, start, special, bspos);
-	    /* end of argument (main closing quote-char) */
-	    Tcl_DStringAppend(&ds, "\"", 1);
+	    if (quote & 4) {
+		/* rest of argument (and escape backslashes before closing main quote) */
+		QuoteCmdLineBackslash(&ds, start, special, bspos);
+		/* end of argument (main closing quote-char) */
+		Tcl_DStringAppend(&ds, "\"", 1);
+	    } else {
+		/* rest of argument */
+		QuoteCmdLineBackslash(&ds, start, special, NULL);
+	    }
 	}
     }
     Tcl_DStringFree(linePtr);