git.vger.kernel.org archive mirror
 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 14/15] chainlint.sed: swallow comments consistently
Date: Mon, 13 Dec 2021 01:30:58 -0500	[thread overview]
Message-ID: <20211213063059.19424-15-sunshine@sunshineco.com> (raw)
In-Reply-To: <20211213063059.19424-1-sunshine@sunshineco.com>

When checking for broken a &&-chain, chainlint.sed knows that the final
statement in a subshell should not end with `&&`, so it takes care to
make a distinction between the final line which is an actual statement
and any lines which may be mere comments preceding the closing ')'. As
such, it swallows comment lines so that they do not interfere with the
&&-chain check.

However, since `sed` does not provide any sort of real recursion,
chainlint.sed only checks &&-chains in subshells one level deep; it
doesn't do any checking in deeper subshells or in `{...}` blocks within
subshells. Furthermore, on account of potential implementation
complexity, it doesn't check &&-chains within `case` arms.

Due to an oversight, it also doesn't swallow comments inside deep
subshells, `{...}` blocks, or `case` statements, which makes its output
inconsistent (swallowing comments in some cases but not others).
Unfortunately, this inconsistency seeps into the chainlint self-test
"expect" files, which potentially makes it difficult to reuse the
self-tests should a more capable chainlint ever be developed. Therefore,
teach chainlint.sed to consistently swallow comments in all cases.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/chainlint.sed                            | 21 +++++++++++++++++++--
 t/chainlint/block-comment.expect           |  6 ++++++
 t/chainlint/block-comment.test             |  8 ++++++++
 t/chainlint/case-comment.expect            |  8 ++++++++
 t/chainlint/case-comment.test              | 11 +++++++++++
 t/chainlint/nested-subshell-comment.expect |  2 --
 6 files changed, 52 insertions(+), 4 deletions(-)
 create mode 100644 t/chainlint/block-comment.expect
 create mode 100644 t/chainlint/block-comment.test
 create mode 100644 t/chainlint/case-comment.expect
 create mode 100644 t/chainlint/case-comment.test

diff --git a/t/chainlint.sed b/t/chainlint.sed
index 0e575c0c62..b1505ef2cd 100644
--- a/t/chainlint.sed
+++ b/t/chainlint.sed
@@ -294,6 +294,12 @@ bfolded
 x
 s/?!HERE?!/<</g
 n
+:cascom
+/^[ 	]*#/{
+	N
+	s/.*\n//
+	bcascom
+}
 /^[ 	]*esac/bslurp
 bcase
 
@@ -322,10 +328,15 @@ x
 :nstslrp
 s/?!HERE?!/<</g
 n
+:nstcom
+# comment -- not closing ")" if in comment
+/^[ 	]*#/{
+	N
+	s/.*\n//
+	bnstcom
+}
 # closing ")" on own line -- stop nested slurp
 /^[ 	]*)/bnstcl
-# comment -- not closing ")" if in comment
-/^[ 	]*#/bnstcnt
 # "$((...))" -- arithmetic expansion; not closing ")"
 /\$(([^)][^)]*))[^)]*$/bnstcnt
 # "$(...)" -- command substitution; not closing ")"
@@ -345,6 +356,12 @@ bchkchn
 x
 s/?!HERE?!/<</g
 n
+:blkcom
+/^[ 	]*#/{
+	N
+	s/.*\n//
+	bblkcom
+}
 # closing "}" -- stop block slurp
 /}/bchkchn
 bblock
diff --git a/t/chainlint/block-comment.expect b/t/chainlint/block-comment.expect
new file mode 100644
index 0000000000..d10b2eeaf2
--- /dev/null
+++ b/t/chainlint/block-comment.expect
@@ -0,0 +1,6 @@
+(
+	{
+		echo a &&
+		echo b
+	}
+)
diff --git a/t/chainlint/block-comment.test b/t/chainlint/block-comment.test
new file mode 100644
index 0000000000..df2beea888
--- /dev/null
+++ b/t/chainlint/block-comment.test
@@ -0,0 +1,8 @@
+(
+	{
+		# show a
+		echo a &&
+		# show b
+		echo b
+	}
+)
diff --git a/t/chainlint/case-comment.expect b/t/chainlint/case-comment.expect
new file mode 100644
index 0000000000..1e4b054bda
--- /dev/null
+++ b/t/chainlint/case-comment.expect
@@ -0,0 +1,8 @@
+(
+	case "$x" in
+	x) foo ;;
+	*)
+		bar
+		;;
+	esac
+)
diff --git a/t/chainlint/case-comment.test b/t/chainlint/case-comment.test
new file mode 100644
index 0000000000..641c157b98
--- /dev/null
+++ b/t/chainlint/case-comment.test
@@ -0,0 +1,11 @@
+(
+	case "$x" in
+	# found foo
+	x) foo ;;
+	# found other
+	*)
+		# treat it as bar
+		bar
+		;;
+	esac
+)
diff --git a/t/chainlint/nested-subshell-comment.expect b/t/chainlint/nested-subshell-comment.expect
index 9138cf386d..be4b27a305 100644
--- a/t/chainlint/nested-subshell-comment.expect
+++ b/t/chainlint/nested-subshell-comment.expect
@@ -2,8 +2,6 @@
 	foo &&
 	(
 		bar &&
-		# bottles wobble while fiddles gobble
-		# minor numbers of cows (or do they?)
 		baz &&
 		snaff
 	) ?!AMP?!
-- 
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 ` [PATCH 09/15] chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?! Eric Sunshine
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 ` Eric Sunshine [this message]
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-15-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).