summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar Eric Sunshine <sunshine@sunshineco.com>2021-12-13 01:30:53 -0500
committerLibravatar Junio C Hamano <gitster@pobox.com>2021-12-13 14:15:29 -0800
commit0d7131763e7762dc34c407f32e3c63123001a292 (patch)
tree8c1e71c5fba52e368341227aad496fd37a2926a5
parentchainlint.sed: tolerate harmless ";" at end of last line in block (diff)
downloadtgif-0d7131763e7762dc34c407f32e3c63123001a292.tar.xz
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>
-rw-r--r--t/chainlint.sed21
-rw-r--r--t/chainlint/negated-one-liner.expect4
-rw-r--r--t/chainlint/one-liner.expect6
-rw-r--r--t/chainlint/semicolon.expect10
-rw-r--r--t/chainlint/subshell-one-liner.expect8
5 files changed, 24 insertions, 25 deletions
diff --git a/t/chainlint.sed b/t/chainlint.sed
index 91077b6e26..f5fcca09ca 100644
--- 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 60baf84b7a..ad4c2d949e 100644
--- 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 3b46554728..57a7a444c1 100644
--- 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 0e6389f532..54a08ce582 100644
--- 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 432217801b..4b44632b09 100644
--- 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
>)