chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?!

>From inception, when chainlint.sed encountered a line using semicolon to
separate commands rather than `&&`, it would insert a ?!SEMI?!
annotation at the beginning of the line rather ?!AMP?! even though the
&&-chain is also broken by the semicolon. Given a line such as:

    ?!SEMI?! cmd1; cmd2 &&

the ?!SEMI?! annotation makes it easier to see what the problem is than
if the output had been:

    ?!AMP?! cmd1; cmd2 &&

which might confuse the test author into thinking that the linter is
broken (since the line clearly ends with `&&`).

However, now that the ?!AMP?! an ?!SEMI?! annotations are inserted at
the point of breakage rather than at the beginning of the line, and
taking into account that both represent a broken &&-chain, there is
little reason to distinguish between the two. Using ?!AMP?! alone is
sufficient to point the test author at the problem. For instance, in:

    cmd1; ?!AMP?! cmd2 &&
    cmd3

it is clear that the &&-chain is broken between `cmd1` and `cmd2`.
Likewise, in:

    cmd1 && cmd2 ?!AMP?!
    cmd3

it is clear that the &&-chain is broken between `cmd2` and `cmd3`.
Finally, in:

    cmd1; ?!AMP?! cmd2 ?!AMP?!
    cmd3

it is clear that the &&-chain is broken between each command.

Hence, there is no longer a good reason to make a distinction between a
broken &&-chain due to a semicolon and a broken chain due to a missing
`&&` at end-of-line. Therefore, drop the ?!SEMI?! annotation and use
?!AMP?! exclusively.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/chainlint.sed b/t/chainlint.sed
index 91077b6e26f9e0500782f810ff478c0cb9c0c321..f5fcca09ca5d55f2b2edfc9ef1b58d317c7c4d39 100644 (file)
--- a/t/chainlint.sed
+++ b/t/chainlint.sed
@@ -24,9 +24,9 @@
 # in order to avoid misinterpreting the ")" in constructs such as "x=$(...)"
 # and "case $x in *)" as ending the subshell.
 #
-# Lines missing a final "&&" are flagged with "?!AMP?!", and lines which chain
-# commands with ";" internally rather than "&&" are flagged "?!SEMI?!". A line
-# may be flagged for both violations.
+# Lines missing a final "&&" are flagged with "?!AMP?!", as are lines which
+# chain commands with ";" internally rather than "&&". A line may be flagged
+# for both violations.
 #
 # Detection of a missing &&-link in a multi-line subshell is complicated by the
 # fact that the last statement before the closing ")" must not end with "&&".
@@ -47,9 +47,8 @@
 # "?!AMP?!" violation is removed from the "bar" line (retrieved from the "hold"
 # area) since the final statement of a subshell must not end with "&&". The
 # final line of a subshell may still break the &&-chain by using ";" internally
-# to chain commands together rather than "&&", so "?!SEMI?!" is not removed
-# from such a line; however, if the line ends with "?!SEMI?!", then the ";" is
-# harmless and the annotation is removed.
+# to chain commands together rather than "&&", but an internal "?!AMP?!" is
+# never removed from a line even though a line-ending "?!AMP?!" might be.
 #
 # Care is taken to recognize the last _statement_ of a multi-line subshell, not
 # necessarily the last textual _line_ within the subshell, since &&-chaining
@@ -127,7 +126,7 @@ b
 # "&&" (but not ";" in a string)
 :oneline
 /;/{
-       /"[^"]*;[^"]*"/!s/;/; ?!SEMI?!/
+       /"[^"]*;[^"]*"/!s/;/; ?!AMP?!/
 }
 b
 
@@ -231,7 +230,7 @@ s/.*\n//
 # string and not ";;" in one-liner "case...esac")
 /;/{
        /;;/!{
-               /"[^"]*;[^"]*"/!s/;/; ?!SEMI?!/
+               /"[^"]*;[^"]*"/!s/;/; ?!AMP?!/
        }
 }
 # line ends with pipe "...|" -- valid; not missing "&&"
@@ -304,7 +303,7 @@ bcase
 # that line legitimately lacks "&&"
 :else
 x
-s/\( ?!SEMI?!\)* ?!AMP?!$//
+s/\( ?!AMP?!\)* ?!AMP?!$//
 x
 bcont
 
@@ -312,7 +311,7 @@ bcont
 # "suspect" from final contained line since that line legitimately lacks "&&"
 :done
 x
-s/\( ?!SEMI?!\)* ?!AMP?!$//
+s/\( ?!AMP?!\)* ?!AMP?!$//
 x
 # is 'done' or 'fi' cuddled with ")" to close subshell?
 /done.*)/bclose
@@ -355,7 +354,7 @@ bblock
 # since that line legitimately lacks "&&" and exit subshell loop
 :clssolo
 x
-s/\( ?!SEMI?!\)* ?!AMP?!$//
+s/\( ?!AMP?!\)* ?!AMP?!$//
 p
 x
 s/^/>/

diff --git a/t/chainlint/negated-one-liner.expect b/t/chainlint/negated-one-liner.expect
index 60baf84b7a621066c333d2f0451f4ce85db70915..ad4c2d949ebf5de221876bd83795c6caee1a4aa2 100644 (file)
--- a/t/chainlint/negated-one-liner.expect
+++ b/t/chainlint/negated-one-liner.expect
@@ -1,5 +1,5 @@
 ! (foo && bar) &&
 ! (foo && bar) >baz &&
 
-! (foo; ?!SEMI?! bar) &&
-! (foo; ?!SEMI?! bar) >baz
+! (foo; ?!AMP?! bar) &&
+! (foo; ?!AMP?! bar) >baz

diff --git a/t/chainlint/one-liner.expect b/t/chainlint/one-liner.expect
index 3b46554728ac2a5a62510a4d9b873f4b4e7191dd..57a7a444c15033cd817502ea4ce9c4b833567acb 100644 (file)
--- a/t/chainlint/one-liner.expect
+++ b/t/chainlint/one-liner.expect
@@ -2,8 +2,8 @@
 (foo && bar) |
 (foo && bar) >baz &&
 
-(foo; ?!SEMI?! bar) &&
-(foo; ?!SEMI?! bar) |
-(foo; ?!SEMI?! bar) >baz &&
+(foo; ?!AMP?! bar) &&
+(foo; ?!AMP?! bar) |
+(foo; ?!AMP?! bar) >baz &&
 
 (foo "bar; baz")

diff --git a/t/chainlint/semicolon.expect b/t/chainlint/semicolon.expect
index 0e6389f532d99220e6781a4d97be8a6bd116718e..54a08ce582f1a2391c813edbcdc4e8e7f580011f 100644 (file)
--- a/t/chainlint/semicolon.expect
+++ b/t/chainlint/semicolon.expect
@@ -1,14 +1,14 @@
 (
-       cat foo ; ?!SEMI?! echo bar ?!AMP?!
-       cat foo ; ?!SEMI?! echo bar
+       cat foo ; ?!AMP?! echo bar ?!AMP?!
+       cat foo ; ?!AMP?! echo bar
 >) &&
 (
-       cat foo ; ?!SEMI?! echo bar &&
-       cat foo ; ?!SEMI?! echo bar
+       cat foo ; ?!AMP?! echo bar &&
+       cat foo ; ?!AMP?! echo bar
 >) &&
 (
        echo "foo; bar" &&
-       cat foo; ?!SEMI?! echo bar
+       cat foo; ?!AMP?! echo bar
 >) &&
 (
        foo;

diff --git a/t/chainlint/subshell-one-liner.expect b/t/chainlint/subshell-one-liner.expect
index 432217801b9b073995ed2f8f71cdf7177d263319..4b44632b099635ed44014c9727b0b0b0047b663e 100644 (file)
--- a/t/chainlint/subshell-one-liner.expect
+++ b/t/chainlint/subshell-one-liner.expect
@@ -2,13 +2,13 @@
        (foo && bar) &&
        (foo && bar) |
        (foo && bar) >baz &&
-       (foo; ?!SEMI?! bar) &&
-       (foo; ?!SEMI?! bar) |
-       (foo; ?!SEMI?! bar) >baz &&
+       (foo; ?!AMP?! bar) &&
+       (foo; ?!AMP?! bar) |
+       (foo; ?!AMP?! bar) >baz &&
        (foo || exit 1) &&
        (foo || exit 1) |
        (foo || exit 1) >baz &&
        (foo && bar) ?!AMP?!
-       (foo && bar; ?!SEMI?! baz) ?!AMP?!
+       (foo && bar; ?!AMP?! baz) ?!AMP?!
        foobar
 >)