git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 14/15] chainlint.sed: swallow comments consistently

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Dec 13, 2021, 06:30 UTC
Message-ID
<20211213063059.19424-15-sunshine@sunshineco.com>
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
Previous: Eric SunshineNext: Eric Sunshine
Message 31 of 34 in “generalize chainlint self-tests”
  1. 00/15 generalize chainlint self-testsEric Sunshine, Dec 13, 2021
  2. 01/15 t/chainlint/*.test: don't use invalid shell syntaxEric Sunshine, Dec 13, 2021
  3. 03/15 t/chainlint/*.test: generalize self-test commentaryEric Sunshine, Dec 13, 2021
  4. 02/15 t/chainlint/*.test: fix invalid test cases due to mixing quote typesEric Sunshine, Dec 13, 2021
  5. 04/15 t/chainlint/one-liner: avoid overly intimate chainlint.sed knowledgeEric Sunshine, Dec 13, 2021
  6. 05/15 t/Makefile: optimize chainlint self-testEric Sunshine, Dec 13, 2021
  7. Fabian StelzerDec 13, 2021
  8. Eric SunshineDec 13, 2021
  9. Fabian StelzerDec 13, 2021
  10. Eric SunshineDec 13, 2021
  11. Fabian StelzerDec 13, 2021
  12. Ævar Arnfjörð BjarmasonDec 13, 2021
  13. Eric SunshineDec 13, 2021
  14. Eric SunshineDec 13, 2021
  15. Ævar Arnfjörð BjarmasonDec 13, 2021
  16. Eric SunshineDec 13, 2021
  17. Ævar Arnfjörð BjarmasonDec 16, 2021
  18. Eric SunshineDec 16, 2021
  19. Ævar Arnfjörð BjarmasonDec 16, 2021
  20. t/Makefile: use dependency graph for "check-chainlint"Ævar Arnfjörð Bjarmason, Dec 13, 2021
  21. Eric SunshineDec 14, 2021
  22. Ævar Arnfjörð BjarmasonDec 14, 2021
  23. 06/15 chainlint.sed: improve ?!AMP?! placement accuracyEric Sunshine, Dec 13, 2021
  24. 07/15 chainlint.sed: improve ?!SEMI?! placement accuracyEric Sunshine, Dec 13, 2021
  25. 08/15 chainlint.sed: tolerate harmless ";" at end of last line in blockEric Sunshine, Dec 13, 2021
  26. 09/15 chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?!Eric Sunshine, Dec 13, 2021
  27. 10/15 chainlint.sed: drop subshell-closing ">" annotationEric Sunshine, Dec 13, 2021
  28. 11/15 chainlint.sed: make here-doc "<<-" operator recognition more POSIX-likeEric Sunshine, Dec 13, 2021
  29. 12/15 chainlint.sed: don't mistake `<< word` in string as here-doc operatorEric Sunshine, Dec 13, 2021
  30. 13/15 chainlint.sed: stop throwing away here-doc tagsEric Sunshine, Dec 13, 2021
  31. 14/15 chainlint.sed: swallow comments consistentlyEric Sunshine, Dec 13, 2021
  32. 15/15 chainlint.sed: stop splitting "(..." into separate lines "(" and "..."Eric Sunshine, Dec 13, 2021
  33. Elijah NewrenDec 15, 2021
  34. Eric SunshineDec 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.