All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>,
	Fabian Stelzer <fs@gigacodes.de>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH 09/15] chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?!
Date: Mon, 13 Dec 2021 01:30:53 -0500	[thread overview]
Message-ID: <20211213063059.19424-10-sunshine@sunshineco.com> (raw)
In-Reply-To: <20211213063059.19424-1-sunshine@sunshineco.com>

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>
---
 t/chainlint.sed                       | 21 ++++++++++-----------
 t/chainlint/negated-one-liner.expect  |  4 ++--
 t/chainlint/one-liner.expect          |  6 +++---
 t/chainlint/semicolon.expect          | 10 +++++-----
 t/chainlint/subshell-one-liner.expect |  8 ++++----
 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
 >)
-- 
2.34.1.397.gfae76fe5da


  parent reply	other threads:[~2021-12-13  6:32 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13  6:30 [PATCH 00/15] generalize chainlint self-tests Eric Sunshine
2021-12-13  6:30 ` [PATCH 01/15] t/chainlint/*.test: don't use invalid shell syntax Eric Sunshine
2021-12-13  6:30 ` [PATCH 02/15] t/chainlint/*.test: fix invalid test cases due to mixing quote types Eric Sunshine
2021-12-13  6:30 ` [PATCH 03/15] t/chainlint/*.test: generalize self-test commentary Eric Sunshine
2021-12-13  6:30 ` [PATCH 04/15] t/chainlint/one-liner: avoid overly intimate chainlint.sed knowledge Eric Sunshine
2021-12-13  6:30 ` [PATCH 05/15] t/Makefile: optimize chainlint self-test Eric Sunshine
2021-12-13 10:09   ` [RFC PATCH] t/Makefile: use dependency graph for "check-chainlint" Ævar Arnfjörð Bjarmason
2021-12-14  7:44     ` Eric Sunshine
2021-12-14 12:34       ` Ævar Arnfjörð Bjarmason
2021-12-13 10:22   ` [PATCH 05/15] t/Makefile: optimize chainlint self-test Fabian Stelzer
2021-12-13 14:27     ` Eric Sunshine
2021-12-13 15:43       ` Fabian Stelzer
2021-12-13 16:02         ` Eric Sunshine
2021-12-13 16:11           ` Ævar Arnfjörð Bjarmason
2021-12-13 17:05             ` Eric Sunshine
2021-12-13 17:25               ` Eric Sunshine
2021-12-13 19:33                 ` Ævar Arnfjörð Bjarmason
2021-12-13 21:37                   ` Eric Sunshine
2021-12-13 16:14           ` Fabian Stelzer
2021-12-16 13:17         ` Ævar Arnfjörð Bjarmason
2021-12-16 15:47           ` Eric Sunshine
2021-12-16 19:26             ` Ævar Arnfjörð Bjarmason
2021-12-13  6:30 ` [PATCH 06/15] chainlint.sed: improve ?!AMP?! placement accuracy Eric Sunshine
2021-12-13  6:30 ` [PATCH 07/15] chainlint.sed: improve ?!SEMI?! " Eric Sunshine
2021-12-13  6:30 ` [PATCH 08/15] chainlint.sed: tolerate harmless ";" at end of last line in block Eric Sunshine
2021-12-13  6:30 ` Eric Sunshine [this message]
2021-12-13  6:30 ` [PATCH 10/15] chainlint.sed: drop subshell-closing ">" annotation Eric Sunshine
2021-12-13  6:30 ` [PATCH 11/15] chainlint.sed: make here-doc "<<-" operator recognition more POSIX-like Eric Sunshine
2021-12-13  6:30 ` [PATCH 12/15] chainlint.sed: don't mistake `<< word` in string as here-doc operator Eric Sunshine
2021-12-13  6:30 ` [PATCH 13/15] chainlint.sed: stop throwing away here-doc tags Eric Sunshine
2021-12-13  6:30 ` [PATCH 14/15] chainlint.sed: swallow comments consistently Eric Sunshine
2021-12-13  6:30 ` [PATCH 15/15] chainlint.sed: stop splitting "(..." into separate lines "(" and "..." Eric Sunshine
2021-12-15  0:00 ` [PATCH 00/15] generalize chainlint self-tests Elijah Newren
2021-12-15  3:15   ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211213063059.19424-10-sunshine@sunshineco.com \
    --to=sunshine@sunshineco.com \
    --cc=fs@gigacodes.de \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.