Skip to content

Commit 0d71317

Browse files
sunshinecogitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3865a7e commit 0d71317

File tree

5 files changed

+24
-25
lines changed

5 files changed

+24
-25
lines changed

t/chainlint.sed

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
# in order to avoid misinterpreting the ")" in constructs such as "x=$(...)"
2525
# and "case $x in *)" as ending the subshell.
2626
#
27-
# Lines missing a final "&&" are flagged with "?!AMP?!", and lines which chain
28-
# commands with ";" internally rather than "&&" are flagged "?!SEMI?!". A line
29-
# may be flagged for both violations.
27+
# Lines missing a final "&&" are flagged with "?!AMP?!", as are lines which
28+
# chain commands with ";" internally rather than "&&". A line may be flagged
29+
# for both violations.
3030
#
3131
# Detection of a missing &&-link in a multi-line subshell is complicated by the
3232
# fact that the last statement before the closing ")" must not end with "&&".
@@ -47,9 +47,8 @@
4747
# "?!AMP?!" violation is removed from the "bar" line (retrieved from the "hold"
4848
# area) since the final statement of a subshell must not end with "&&". The
4949
# final line of a subshell may still break the &&-chain by using ";" internally
50-
# to chain commands together rather than "&&", so "?!SEMI?!" is not removed
51-
# from such a line; however, if the line ends with "?!SEMI?!", then the ";" is
52-
# harmless and the annotation is removed.
50+
# to chain commands together rather than "&&", but an internal "?!AMP?!" is
51+
# never removed from a line even though a line-ending "?!AMP?!" might be.
5352
#
5453
# Care is taken to recognize the last _statement_ of a multi-line subshell, not
5554
# necessarily the last textual _line_ within the subshell, since &&-chaining
@@ -127,7 +126,7 @@ b
127126
# "&&" (but not ";" in a string)
128127
:oneline
129128
/;/{
130-
/"[^"]*;[^"]*"/!s/;/; ?!SEMI?!/
129+
/"[^"]*;[^"]*"/!s/;/; ?!AMP?!/
131130
}
132131
b
133132

@@ -231,7 +230,7 @@ s/.*\n//
231230
# string and not ";;" in one-liner "case...esac")
232231
/;/{
233232
/;;/!{
234-
/"[^"]*;[^"]*"/!s/;/; ?!SEMI?!/
233+
/"[^"]*;[^"]*"/!s/;/; ?!AMP?!/
235234
}
236235
}
237236
# line ends with pipe "...|" -- valid; not missing "&&"
@@ -304,15 +303,15 @@ bcase
304303
# that line legitimately lacks "&&"
305304
:else
306305
x
307-
s/\( ?!SEMI?!\)* ?!AMP?!$//
306+
s/\( ?!AMP?!\)* ?!AMP?!$//
308307
x
309308
bcont
310309

311310
# found "done" closing for-loop or while-loop, or "fi" closing if-then -- drop
312311
# "suspect" from final contained line since that line legitimately lacks "&&"
313312
:done
314313
x
315-
s/\( ?!SEMI?!\)* ?!AMP?!$//
314+
s/\( ?!AMP?!\)* ?!AMP?!$//
316315
x
317316
# is 'done' or 'fi' cuddled with ")" to close subshell?
318317
/done.*)/bclose
@@ -355,7 +354,7 @@ bblock
355354
# since that line legitimately lacks "&&" and exit subshell loop
356355
:clssolo
357356
x
358-
s/\( ?!SEMI?!\)* ?!AMP?!$//
357+
s/\( ?!AMP?!\)* ?!AMP?!$//
359358
p
360359
x
361360
s/^/>/

t/chainlint/negated-one-liner.expect

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
! (foo && bar) &&
22
! (foo && bar) >baz &&
33

4-
! (foo; ?!SEMI?! bar) &&
5-
! (foo; ?!SEMI?! bar) >baz
4+
! (foo; ?!AMP?! bar) &&
5+
! (foo; ?!AMP?! bar) >baz

t/chainlint/one-liner.expect

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
(foo && bar) |
33
(foo && bar) >baz &&
44

5-
(foo; ?!SEMI?! bar) &&
6-
(foo; ?!SEMI?! bar) |
7-
(foo; ?!SEMI?! bar) >baz &&
5+
(foo; ?!AMP?! bar) &&
6+
(foo; ?!AMP?! bar) |
7+
(foo; ?!AMP?! bar) >baz &&
88

99
(foo "bar; baz")

t/chainlint/semicolon.expect

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
(
2-
cat foo ; ?!SEMI?! echo bar ?!AMP?!
3-
cat foo ; ?!SEMI?! echo bar
2+
cat foo ; ?!AMP?! echo bar ?!AMP?!
3+
cat foo ; ?!AMP?! echo bar
44
>) &&
55
(
6-
cat foo ; ?!SEMI?! echo bar &&
7-
cat foo ; ?!SEMI?! echo bar
6+
cat foo ; ?!AMP?! echo bar &&
7+
cat foo ; ?!AMP?! echo bar
88
>) &&
99
(
1010
echo "foo; bar" &&
11-
cat foo; ?!SEMI?! echo bar
11+
cat foo; ?!AMP?! echo bar
1212
>) &&
1313
(
1414
foo;

t/chainlint/subshell-one-liner.expect

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
(foo && bar) &&
33
(foo && bar) |
44
(foo && bar) >baz &&
5-
(foo; ?!SEMI?! bar) &&
6-
(foo; ?!SEMI?! bar) |
7-
(foo; ?!SEMI?! bar) >baz &&
5+
(foo; ?!AMP?! bar) &&
6+
(foo; ?!AMP?! bar) |
7+
(foo; ?!AMP?! bar) >baz &&
88
(foo || exit 1) &&
99
(foo || exit 1) |
1010
(foo || exit 1) >baz &&
1111
(foo && bar) ?!AMP?!
12-
(foo && bar; ?!SEMI?! baz) ?!AMP?!
12+
(foo && bar; ?!AMP?! baz) ?!AMP?!
1313
foobar
1414
>)

0 commit comments

Comments
 (0)