{"thread":{"id":"58777","subject":"[PATCH 0/3] chainlint: emit line numbers alongside test definitions","startedAt":"2022-11-09T17:01:42Z","lastAt":"2022-11-11T21:57:05Z","messageCount":20,"participants":["Eric Sunshine via GitGitGadget","Taylor Blau","brian m. carlson","Eric Sunshine","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"466969","messageId":"pull.1413.git.1668013114.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":null,"subject":"[PATCH 0/3] chainlint: emit line numbers alongside test definitions","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-09T16:58:31Z","receivedAt":"2022-11-09T17:01:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When chainlint detects problems in a test, it prints out the name of the\ntest script, the name of the problematic test, and a copy of the test\ndefinition with \"?!FOO?!\" annotations inserted at the locations where\nproblems were detected. Taken together this information is sufficient for\nthe test author to identify the problematic code in the original test\ndefinition. However, in a lengthy script or a lengthy test definition, the\nauthor may still end up using the editor's search feature to home in on the\nexact problem location.\n\nThis patch series further assists the test author by displaying line numbers\nalongside the annotated test definition, thus allowing the author to jump\ndirectly to each problematic line.\n\nThis feature was suggested by Ævar[1]. I suspect that Ævar's next nerd-snipe\nattempt may be to have problems emitted in \"path:line#:col#: message\" format\nto allow editors to jump directly to the problem without the user having to\ntype in the line number manually.\n\nThis is atop \"es/chainlint-output\"[2].\n\n(Note to self: Fortify against Ævar's nerd-snipe blacklist evasion.)\n\nFOOTNOTES\n\n[1] https://lore.kernel.org/git/221108.86iljpqdvj.gmgdl@evledraar.gmail.com/\n[2]\nhttps://lore.kernel.org/git/pull.1375.git.git.1667934510.gitgitgadget@gmail.com/\n\nEric Sunshine (3):\n  chainlint: sidestep impoverished macOS \"terminfo\"\n  chainlint: latch line numbers at which each token starts and ends\n  chainlint: prefix annotated test definition with line numbers\n\n t/Makefile     |  2 +-\n t/chainlint.pl | 69 ++++++++++++++++++++++++++++++++++----------------\n 2 files changed, 48 insertions(+), 23 deletions(-)\n\n\nbase-commit: 73c768dae9ea4838736693965b25ba34e941ac88\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1413%2Fsunshineco%2Fchainlintline-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1413/sunshineco/chainlintline-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1413\n-- \ngitgitgadget\n"},{"id":"466970","messageId":"b85b28e5a6beea97c149f0b9de6ba8d0a4a7c1f9.1668013114.git.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.git.1668013114.gitgitgadget@gmail.com","subject":"[PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-09T16:58:32Z","receivedAt":"2022-11-09T17:01:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nAlthough the macOS Terminal.app is \"xterm\"-compatible, its corresponding\n\"terminfo\" entry neglects to mention capabilities which Terminal.app\nactually supports (such as \"dim text\"). This oversight on Apple's part\nends up penalizing users of \"good citizen\" console programs which\nconsult \"terminfo\" to tailor their output based upon reported terminal\ncapabilities (as opposed to programs which assume that the terminal\nsupports ANSI codes).\n\nSidestep this Apple problem by imbuing get_colors() with specific\nknowledge of \"xterm\" capabilities rather than trusting \"terminfo\" to\nreport them correctly. Although hard-coding such knowledge is ugly,\n\"xterm\" support is nearly ubiquitous these days, and Git itself sets\nprecedence by assuming support for ANSI color codes. For non-\"xterm\",\nfall back to querying \"terminfo\" via `tput` as usual.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/chainlint.pl | 35 +++++++++++++++++++++++------------\n 1 file changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/t/chainlint.pl b/t/chainlint.pl\nindex 7972c5bbe6f..fcf4d459249 100755\n--- a/t/chainlint.pl\n+++ b/t/chainlint.pl\n@@ -653,21 +653,32 @@ my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '', green => '', red\n my %COLORS = ();\n sub get_colors {\n \treturn \\%COLORS if %COLORS;\n-\tif (exists($ENV{NO_COLOR}) ||\n-\t    system(\"tput sgr0 >/dev/null 2>&1\") != 0 ||\n-\t    system(\"tput bold >/dev/null 2>&1\") != 0 ||\n-\t    system(\"tput rev  >/dev/null 2>&1\") != 0 ||\n-\t    system(\"tput setaf 1 >/dev/null 2>&1\") != 0) {\n+\tif (exists($ENV{NO_COLOR})) {\n \t\t%COLORS = @NOCOLORS;\n \t\treturn \\%COLORS;\n \t}\n-\t%COLORS = (bold  => `tput bold`,\n-\t\t   rev   => `tput rev`,\n-\t\t   reset => `tput sgr0`,\n-\t\t   blue  => `tput setaf 4`,\n-\t\t   green => `tput setaf 2`,\n-\t\t   red   => `tput setaf 1`);\n-\tchomp(%COLORS);\n+\tif ($ENV{TERM} =~ /\\bxterm\\b/) {\n+\t\t%COLORS = (bold  => \"\\e[1m\",\n+\t\t\t   rev   => \"\\e[7m\",\n+\t\t\t   reset => \"\\e[0m\",\n+\t\t\t   blue  => \"\\e[34m\",\n+\t\t\t   green => \"\\e[32m\",\n+\t\t\t   red   => \"\\e[31m\");\n+\t\treturn \\%COLORS;\n+\t}\n+\tif (system(\"tput sgr0 >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput bold >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput rev  >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput setaf 1 >/dev/null 2>&1\") == 0) {\n+\t\t%COLORS = (bold  => `tput bold`,\n+\t\t\t   rev   => `tput rev`,\n+\t\t\t   reset => `tput sgr0`,\n+\t\t\t   blue  => `tput setaf 4`,\n+\t\t\t   green => `tput setaf 2`,\n+\t\t\t   red   => `tput setaf 1`);\n+\t\treturn \\%COLORS;\n+\t}\n+\t%COLORS = @NOCOLORS;\n \treturn \\%COLORS;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"466971","messageId":"c8a316426be4cefb6382f524a89226c76d6d1d97.1668013114.git.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.git.1668013114.gitgitgadget@gmail.com","subject":"[PATCH 2/3] chainlint: latch line numbers at which each token starts and ends","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-09T16:58:33Z","receivedAt":"2022-11-09T17:01:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nWhen chainlint detects problems in a test, it prints out the name of the\ntest script, the name of the problematic test, and a copy of the test\ndefinition with \"?!FOO?!\" annotations inserted at the locations where\nproblems were detected. Taken together this information is sufficient\nfor the test author to identify the problematic code in the original\ntest definition. However, in a lengthy script or a lengthy test\ndefinition, the author may still end up using the editor's search\nfeature to home in on the exact problem location.\n\nTo further assist the test author, an upcoming change will display line\nnumbers along with the annotated test definition, thus allowing the\nauthor to jump directly to each problematic line. As preparation,\nupgrade Lexer to latch the line numbers at which each token starts and\nends, and return that information with the token itself.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/chainlint.pl | 24 ++++++++++++++++--------\n 1 file changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/t/chainlint.pl b/t/chainlint.pl\nindex fcf4d459249..01f261165b1 100755\n--- a/t/chainlint.pl\n+++ b/t/chainlint.pl\n@@ -67,6 +67,7 @@ sub new {\n \tbless {\n \t\tparser => $parser,\n \t\tbuff => $s,\n+\t\tlineno => 1,\n \t\theretags => []\n \t} => $class;\n }\n@@ -97,7 +98,9 @@ sub scan_op {\n sub scan_sqstring {\n \tmy $self = shift @_;\n \t${$self->{buff}} =~ /\\G([^']*'|.*\\z)/sgc;\n-\treturn \"'\" . $1;\n+\tmy $s = $1;\n+\t$self->{lineno} += () = $s =~ /\\n/sg;\n+\treturn \"'\" . $s;\n }\n \n sub scan_dqstring {\n@@ -115,7 +118,7 @@ sub scan_dqstring {\n \t\tif ($c eq '\\\\') {\n \t\t\t$s .= '\\\\', last unless $$b =~ /\\G(.)/sgc;\n \t\t\t$c = $1;\n-\t\t\tnext if $c eq \"\\n\"; # line splice\n+\t\t\t$self->{lineno}++, next if $c eq \"\\n\"; # line splice\n \t\t\t# backslash escapes only $, `, \", \\ in dq-string\n \t\t\t$s .= '\\\\' unless $c =~ /^[\\$`\"\\\\]$/;\n \t\t\t$s .= $c;\n@@ -123,6 +126,7 @@ sub scan_dqstring {\n \t\t}\n \t\tdie(\"internal error scanning dq-string '$c'\\n\");\n \t}\n+\t$self->{lineno} += () = $s =~ /\\n/sg;\n \treturn $s;\n }\n \n@@ -137,6 +141,7 @@ sub scan_balanced {\n \t\t$depth--;\n \t\tlast if $depth == 0;\n \t}\n+\t$self->{lineno} += () = $s =~ /\\n/sg;\n \treturn $s;\n }\n \n@@ -165,6 +170,8 @@ sub swallow_heredocs {\n \twhile (my $tag = shift @$tags) {\n \t\tmy $indent = $tag =~ s/^\\t// ? '\\\\s*' : '';\n \t\t$$b =~ /(?:\\G|\\n)$indent\\Q$tag\\E(?:\\n|\\z)/gc;\n+\t\tmy $body = $&;\n+\t\t$self->{lineno} += () = $body =~ /\\n/sg;\n \t}\n }\n \n@@ -172,11 +179,12 @@ sub scan_token {\n \tmy $self = shift @_;\n \tmy $b = $self->{buff};\n \tmy $token = '';\n-\tmy $start;\n+\tmy ($start, $startln);\n RESTART:\n+\t$startln = $self->{lineno};\n \t$$b =~ /\\G[ \\t]+/gc; # skip whitespace (but not newline)\n \t$start = pos($$b) || 0;\n-\treturn [\"\\n\", $start, pos($$b)] if $$b =~ /\\G#[^\\n]*(?:\\n|\\z)/gc; # comment\n+\t$self->{lineno}++, return [\"\\n\", $start, pos($$b), $startln, $startln] if $$b =~ /\\G#[^\\n]*(?:\\n|\\z)/gc; # comment\n \twhile (1) {\n \t\t# slurp up non-special characters\n \t\t$token .= $1 if $$b =~ /\\G([^\\\\;&|<>(){}'\"\\$\\s]+)/gc;\n@@ -188,20 +196,20 @@ RESTART:\n \t\t$token .= $self->scan_sqstring(), next if $c eq \"'\";\n \t\t$token .= $self->scan_dqstring(), next if $c eq '\"';\n \t\t$token .= $c . $self->scan_dollar(), next if $c eq '$';\n-\t\t$self->swallow_heredocs(), $token = $c, last if $c eq \"\\n\";\n+\t\t$self->{lineno}++, $self->swallow_heredocs(), $token = $c, last if $c eq \"\\n\";\n \t\t$token = $self->scan_op($c), last if $c =~ /^[;&|<>]$/;\n \t\t$token = $c, last if $c =~ /^[(){}]$/;\n \t\tif ($c eq '\\\\') {\n \t\t\t$token .= '\\\\', last unless $$b =~ /\\G(.)/sgc;\n \t\t\t$c = $1;\n-\t\t\tnext if $c eq \"\\n\" && length($token); # line splice\n-\t\t\tgoto RESTART if $c eq \"\\n\"; # line splice\n+\t\t\t$self->{lineno}++, next if $c eq \"\\n\" && length($token); # line splice\n+\t\t\t$self->{lineno}++, goto RESTART if $c eq \"\\n\"; # line splice\n \t\t\t$token .= '\\\\' . $c;\n \t\t\tnext;\n \t\t}\n \t\tdie(\"internal error scanning character '$c'\\n\");\n \t}\n-\treturn length($token) ? [$token, $start, pos($$b)] : undef;\n+\treturn length($token) ? [$token, $start, pos($$b), $startln, $self->{lineno}] : undef;\n }\n \n # ShellParser parses POSIX shell scripts (with minor extensions for Bash). It\n-- \ngitgitgadget\n\n"},{"id":"466972","messageId":"380b146abd1d97d51511c7acd11ffb99d1affcc6.1668013114.git.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.git.1668013114.gitgitgadget@gmail.com","subject":"[PATCH 3/3] chainlint: prefix annotated test definition with line numbers","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-09T16:58:34Z","receivedAt":"2022-11-09T17:01:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nWhen chainlint detects problems in a test, it prints out the name of the\ntest script, the name of the problematic test, and a copy of the test\ndefinition with \"?!FOO?!\" annotations inserted at the locations where\nproblems were detected. Taken together this information is sufficient\nfor the test author to identify the problematic code in the original\ntest definition. However, in a lengthy script or a lengthy test\ndefinition, the author may still end up using the editor's search\nfeature to home in on the exact problem location.\n\nTo further assist the test author, display line numbers along with the\nannotated test definition, thus allowing the author to jump directly to\neach problematic line.\n\nSuggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/Makefile     |  2 +-\n t/chainlint.pl | 10 ++++++++--\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 882782a519c..2c2b2522402 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -94,7 +94,7 @@ check-chainlint:\n \t\tdone \\\n \t} >'$(CHAINLINTTMP_SQ)'/expect && \\\n \t$(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n-\t\tgrep -v '^[ \t]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \\\n+\t\tsed -e 's/^[1-9][0-9]* //;/^[ \t]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n \tif test -f ../GIT-BUILD-OPTIONS; then \\\n \t\t. ../GIT-BUILD-OPTIONS; \\\n \tfi && \\\ndiff --git a/t/chainlint.pl b/t/chainlint.pl\nindex 01f261165b1..48dde978480 100755\n--- a/t/chainlint.pl\n+++ b/t/chainlint.pl\n@@ -613,6 +613,7 @@ sub check_test {\n \tmy $problems = $parser->{problems};\n \treturn unless $emit_all || @$problems;\n \tmy $c = main::fd_colors(1);\n+\tmy $lineno = $_[1]->[3];\n \tmy $start = 0;\n \tmy $checked = '';\n \tfor (sort {$a->[1]->[2] <=> $b->[1]->[2]} @$problems) {\n@@ -622,10 +623,12 @@ sub check_test {\n \t\t$start = $pos;\n \t}\n \t$checked .= substr($body, $start);\n-\t$checked =~ s/^\\n//;\n+\t$checked =~ s/^/$lineno++ . ' '/mge;\n+\t$checked =~ s/^\\d+ \\n//;\n \t$checked =~ s/(\\s) \\?!/$1?!/mg;\n \t$checked =~ s/\\?! (\\s)/?!$1/mg;\n \t$checked =~ s/(\\?![^?]+\\?!)/$c->{rev}$c->{red}$1$c->{reset}/mg;\n+\t$checked =~ s/^\\d+/$c->{dim}$&$c->{reset}/mg;\n \t$checked .= \"\\n\" unless $checked =~ /\\n$/;\n \tpush(@{$self->{output}}, \"$c->{blue}# chainlint: $title$c->{reset}\\n$checked\");\n }\n@@ -657,7 +660,7 @@ if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) {\n # thread and ignore %ENV changes in subthreads.\n $ENV{TERM} = $ENV{USER_TERM} if $ENV{USER_TERM};\n \n-my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '', green => '', red => '');\n+my @NOCOLORS = (bold => '', rev => '', dim => '', reset => '', blue => '', green => '', red => '');\n my %COLORS = ();\n sub get_colors {\n \treturn \\%COLORS if %COLORS;\n@@ -668,6 +671,7 @@ sub get_colors {\n \tif ($ENV{TERM} =~ /\\bxterm\\b/) {\n \t\t%COLORS = (bold  => \"\\e[1m\",\n \t\t\t   rev   => \"\\e[7m\",\n+\t\t\t   dim   => \"\\e[2m\",\n \t\t\t   reset => \"\\e[0m\",\n \t\t\t   blue  => \"\\e[34m\",\n \t\t\t   green => \"\\e[32m\",\n@@ -677,9 +681,11 @@ sub get_colors {\n \tif (system(\"tput sgr0 >/dev/null 2>&1\") == 0 &&\n \t    system(\"tput bold >/dev/null 2>&1\") == 0 &&\n \t    system(\"tput rev  >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput dim  >/dev/null 2>&1\") == 0 &&\n \t    system(\"tput setaf 1 >/dev/null 2>&1\") == 0) {\n \t\t%COLORS = (bold  => `tput bold`,\n \t\t\t   rev   => `tput rev`,\n+\t\t\t   dim   => `tput dim`,\n \t\t\t   reset => `tput sgr0`,\n \t\t\t   blue  => `tput setaf 4`,\n \t\t\t   green => `tput setaf 2`,\n-- \ngitgitgadget\n"},{"id":"466993","messageId":"Y2wnJ1h7xwyrpRYs@nand.local","threadId":"58777","inReplyTo":"b85b28e5a6beea97c149f0b9de6ba8d0a4a7c1f9.1668013114.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-09T22:18:15Z","receivedAt":"2022-11-09T22:18:26Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Nov 09, 2022 at 04:58:32PM +0000, Eric Sunshine via GitGitGadget wrote:\n> From: Eric Sunshine <sunshine@sunshineco.com>\n>\n> Although the macOS Terminal.app is \"xterm\"-compatible, its corresponding\n> \"terminfo\" entry neglects to mention capabilities which Terminal.app\n> actually supports (such as \"dim text\"). This oversight on Apple's part\n> ends up penalizing users of \"good citizen\" console programs which\n> consult \"terminfo\" to tailor their output based upon reported terminal\n> capabilities (as opposed to programs which assume that the terminal\n> supports ANSI codes).\n\nHmmph. Too bad that Apple isn't doing the right thing here, but your\napproach is reasonable and well-explained. Looking good.\n\nThanks,\nTaylor\n"},{"id":"466995","messageId":"Y2woMwY2CmteZwgS@nand.local","threadId":"58777","inReplyTo":"pull.1413.git.1668013114.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] chainlint: emit line numbers alongside test definitions","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-09T22:22:43Z","receivedAt":"2022-11-09T22:22:48Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Nov 09, 2022 at 04:58:31PM +0000, Eric Sunshine via GitGitGadget wrote:\n> This patch series further assists the test author by displaying line numbers\n> alongside the annotated test definition, thus allowing the author to jump\n> directly to each problematic line.\n\nThis is really nifty. I applied it on top of your earlier series,\nintentionally broke a test and got some very pleasing chainlint output\nafter trying to run it.\n\nAs previously, I am no expert in the chainlint code, but everything here\nlooks pretty reasonable to me. And certainly it works, so I'm inclined\nto start merging this and the other topic down.\n\nIt would be nice to have some more familiar eyes take a look at it,\nthough.\n\n> (Note to self: Fortify against Ævar's nerd-snipe blacklist evasion.)\n\n;-).\n\nThanks,\nTaylor\n"},{"id":"467028","messageId":"Y2xkpJj4jLqfsggL@tapette.crustytoothpaste.net","threadId":"58777","inReplyTo":"b85b28e5a6beea97c149f0b9de6ba8d0a4a7c1f9.1668013114.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-11-10T02:40:36Z","receivedAt":"2022-11-10T02:40:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-11-09 at 16:58:32, Eric Sunshine via GitGitGadget wrote:\n> From: Eric Sunshine <sunshine@sunshineco.com>\n> \n> Although the macOS Terminal.app is \"xterm\"-compatible, its corresponding\n> \"terminfo\" entry neglects to mention capabilities which Terminal.app\n> actually supports (such as \"dim text\"). This oversight on Apple's part\n> ends up penalizing users of \"good citizen\" console programs which\n> consult \"terminfo\" to tailor their output based upon reported terminal\n> capabilities (as opposed to programs which assume that the terminal\n> supports ANSI codes).\n> \n> Sidestep this Apple problem by imbuing get_colors() with specific\n> knowledge of \"xterm\" capabilities rather than trusting \"terminfo\" to\n> report them correctly. Although hard-coding such knowledge is ugly,\n> \"xterm\" support is nearly ubiquitous these days, and Git itself sets\n> precedence by assuming support for ANSI color codes. For non-\"xterm\",\n> fall back to querying \"terminfo\" via `tput` as usual.\n\nGiven the regex below, I think the question here is actually whether\nXTerm itself supports these in all its variants (my Debian system lists\napproximately 90 of them), many of which are quite old.  While I don't\nexpect most of them to see common use, given the interest some people\nhave in retrocomputing, I don't think we can exclude the possibility of\nseeing people use esoteric xterm variants over an SSH (or, perhaps less\npleasantly, telnet) connection.\n\nTerminal.app actually has its own set of terminal types, nsterm*, which\nare properly used here instead, although I realize that most people\nprefer the xterm* options for compatibility and ease of use.  However,\nthat kind of behaviour does result in breakage when the canonical\nterminal for that type (in this case XTerm) implements new features that\naren't supported in other implementations.\n\nPerhaps, instead of auditing all 90 terminal types, we should tighten\nthis to xterm, xterm-256color, and xterm-direct[0]?  That should cover\nthe vast majority of use cases in the real world today, including most\nusers of macOS and Terminal.app, while avoiding breaking some older\nvariants (e.g., xterm-old lacks setaf).\n\n> +\tif ($ENV{TERM} =~ /\\bxterm\\b/) {\n> +\t\t%COLORS = (bold  => \"\\e[1m\",\n> +\t\t\t   rev   => \"\\e[7m\",\n> +\t\t\t   reset => \"\\e[0m\",\n> +\t\t\t   blue  => \"\\e[34m\",\n> +\t\t\t   green => \"\\e[32m\",\n> +\t\t\t   red   => \"\\e[31m\");\n> +\t\treturn \\%COLORS;\n> +\t}\n\n[0] *-direct is what's typically used by ncurses for true colour\nvariants.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"467031","messageId":"CAPig+cTL4x45E2a0RbpO2ntPo08K8hQ2wxcXm=QesqtYqxpvaw@mail.gmail.com","threadId":"58777","inReplyTo":"Y2xkpJj4jLqfsggL@tapette.crustytoothpaste.net","subject":"Re: [PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-11-10T03:37:16Z","receivedAt":"2022-11-10T03:37:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Nov 9, 2022 at 9:40 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On 2022-11-09 at 16:58:32, Eric Sunshine via GitGitGadget wrote:\n> > Sidestep this Apple problem by imbuing get_colors() with specific\n> > knowledge of \"xterm\" capabilities rather than trusting \"terminfo\" to\n> > report them correctly. Although hard-coding such knowledge is ugly,\n> > \"xterm\" support is nearly ubiquitous these days, and Git itself sets\n> > precedence by assuming support for ANSI color codes. For non-\"xterm\",\n> > fall back to querying \"terminfo\" via `tput` as usual.\n>\n> Given the regex below, I think the question here is actually whether\n> XTerm itself supports these in all its variants (my Debian system lists\n> approximately 90 of them), many of which are quite old.  While I don't\n> expect most of them to see common use, given the interest some people\n> have in retrocomputing, I don't think we can exclude the possibility of\n> seeing people use esoteric xterm variants over an SSH (or, perhaps less\n> pleasantly, telnet) connection.\n\nI get your drift, but I have to wonder if the retrocomputing crowd is\nreally going to be crafting Git tests directly on their retrohardware.\n(appropriate emoji here)\n\n> Terminal.app actually has its own set of terminal types, nsterm*, which\n> are properly used here instead, although I realize that most people\n> prefer the xterm* options for compatibility and ease of use.\n\nHmm, on my machine \"nsterm\" also lacks the \"dim\" capability. I see\nthat Neovim docs recommend \"nsterm\" with Terminal.app, so perhaps that\nought to be handled specially here, as well. Do you think any\nvariations other than base \"nsterm\" are worth special-casing?\n\n> Perhaps, instead of auditing all 90 terminal types, we should tighten\n> this to xterm, xterm-256color, and xterm-direct[0]?  That should cover\n> the vast majority of use cases in the real world today, including most\n> users of macOS and Terminal.app, while avoiding breaking some older\n> variants (e.g., xterm-old lacks setaf).\n\nI don't mind tightening which terminal types are handled specially.\n\"xterm-direct\" doesn't exist on my old macOS. Is it present on newer\nmacOS? If so, does it require special-casing (i.e. does it lack\n\"dim\")? If we don't special-case \"xterm-direct\", it will fall back to\nusing `tput` interrogation, which should be fine as long as the\n\"xterm-direct\" terminfo entry is accurate.\n\nI notice that the iTerm2 FAQ also recommends \"xterm-new\" on macOS, and\nthat one lacks \"dim\", as well on my machine. So, it seems that it\nshould be special-cased too.\n\nTaking all the above into account, perhaps this regex?\n\n    /xterm|xterm-.*color|xterm-new|nsterm/\n\nOf course, the other option is to follow Git's own lead by not\nworrying about TERM and `tput` and just assume everyone understands\nANSI color codes. I'm too old-school to feel entirely comfortable with\nthat approach, but I would entertain it if others feel it is safe\nenough.\n"},{"id":"467092","messageId":"Y215ZKz2iZWJCYo3@tapette.crustytoothpaste.net","threadId":"58777","inReplyTo":"CAPig+cTL4x45E2a0RbpO2ntPo08K8hQ2wxcXm=QesqtYqxpvaw@mail.gmail.com","subject":"Re: [PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-11-10T22:21:24Z","receivedAt":"2022-11-10T22:21:31Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-11-10 at 03:37:16, Eric Sunshine wrote:\n> Hmm, on my machine \"nsterm\" also lacks the \"dim\" capability. I see\n> that Neovim docs recommend \"nsterm\" with Terminal.app, so perhaps that\n> ought to be handled specially here, as well. Do you think any\n> variations other than base \"nsterm\" are worth special-casing?\n\nI'd say we should do nsterm, nsterm-256color, and nsterm-direct.\n\n> I don't mind tightening which terminal types are handled specially.\n> \"xterm-direct\" doesn't exist on my old macOS. Is it present on newer\n> macOS? If so, does it require special-casing (i.e. does it lack\n> \"dim\")? If we don't special-case \"xterm-direct\", it will fall back to\n> using `tput` interrogation, which should be fine as long as the\n> \"xterm-direct\" terminfo entry is accurate.\n\nIt's present in newer ncurses, so I expect it will make its way to macOS\neventually.  I don't know whether Apple's version of it will contain\nthe `dim` capability, but on Debian all three xterm variants do.\n\nIt sounds like Apple is specifically limiting their capabilities for\nsome reason when upstream ncurses doesn't.  I can't say why that is, but\nperhaps it's for compatibility.  Debian had to do that for one release\nwith screen* when Screen added support for some new feature but tmux had\nnot.\n\n> I notice that the iTerm2 FAQ also recommends \"xterm-new\" on macOS, and\n> that one lacks \"dim\", as well on my machine. So, it seems that it\n> should be special-cased too.\n> \n> Taking all the above into account, perhaps this regex?\n> \n>     /xterm|xterm-.*color|xterm-new|nsterm/\n\nMaybe this, then?\n\n/(xterm|nsterm)(-(256color|direct))?|xterm-new/\n\nThat matches the three special variants of each one here plus xterm-new.\n\n> Of course, the other option is to follow Git's own lead by not\n> worrying about TERM and `tput` and just assume everyone understands\n> ANSI color codes. I'm too old-school to feel entirely comfortable with\n> that approach, but I would entertain it if others feel it is safe\n> enough.\n\nSure.  I would also prefer to avoid that.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"467093","messageId":"CAPig+cRcw8qxAMTZz-CpM23zPn+VpPJ7qQTtFrbMRmgbiyhymQ@mail.gmail.com","threadId":"58777","inReplyTo":"Y215ZKz2iZWJCYo3@tapette.crustytoothpaste.net","subject":"Re: [PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-11-10T22:36:14Z","receivedAt":"2022-11-10T22:36:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 10, 2022 at 5:21 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On 2022-11-10 at 03:37:16, Eric Sunshine wrote:\n> > I notice that the iTerm2 FAQ also recommends \"xterm-new\" on macOS, and\n> > that one lacks \"dim\", as well on my machine. So, it seems that it\n> > should be special-cased too.\n> >\n> > Taking all the above into account, perhaps this regex?\n> >\n> >     /xterm|xterm-.*color|xterm-new|nsterm/\n>\n> Maybe this, then?\n>\n> /(xterm|nsterm)(-(256color|direct))?|xterm-new/\n>\n> That matches the three special variants of each one here plus xterm-new.\n\nI was thinking of targeting xterm-16color too, not just\nxterm-256color, just to cover bases a bit better.\n\nI also don't mind manually spelling out the regex:\n\n    /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/\n\nfor simplicity's sake; sure it's verbose, but it's also dead-easy for\npeople to understand and extend in the future if necessary.\n"},{"id":"467094","messageId":"Y21/0Kxd2gY2eMBE@tapette.crustytoothpaste.net","threadId":"58777","inReplyTo":"CAPig+cRcw8qxAMTZz-CpM23zPn+VpPJ7qQTtFrbMRmgbiyhymQ@mail.gmail.com","subject":"Re: [PATCH 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2022-11-10T22:48:48Z","receivedAt":"2022-11-10T22:48:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2022-11-10 at 22:36:14, Eric Sunshine wrote:\n> On Thu, Nov 10, 2022 at 5:21 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > On 2022-11-10 at 03:37:16, Eric Sunshine wrote:\n> > > I notice that the iTerm2 FAQ also recommends \"xterm-new\" on macOS, and\n> > > that one lacks \"dim\", as well on my machine. So, it seems that it\n> > > should be special-cased too.\n> > >\n> > > Taking all the above into account, perhaps this regex?\n> > >\n> > >     /xterm|xterm-.*color|xterm-new|nsterm/\n> >\n> > Maybe this, then?\n> >\n> > /(xterm|nsterm)(-(256color|direct))?|xterm-new/\n> >\n> > That matches the three special variants of each one here plus xterm-new.\n> \n> I was thinking of targeting xterm-16color too, not just\n> xterm-256color, just to cover bases a bit better.\n\nSure, that seems like a good idea.  I know that was popular for a time,\nalthough I feel like it's maybe less popular today with more colour\noptions.\n\n> I also don't mind manually spelling out the regex:\n> \n>     /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/\n> \n> for simplicity's sake; sure it's verbose, but it's also dead-easy for\n> people to understand and extend in the future if necessary.\n\nSimplicity is nice.  I think that seems like a good pattern.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"467130","messageId":"pull.1413.v2.git.1668152094.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.git.1668013114.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] chainlint: emit line numbers alongside test definitions","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T07:34:51Z","receivedAt":"2022-11-11T07:35:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This is a re-roll of \"es/chainlint-lineno\"[1] which makes chainlint.pl\noutput line numbers alongside annotated test definitions in which problems\nhave been discovered.\n\nChanges since v1:\n\nLimit sidestepping of impoverished Apple \"terminfo\" entries to specific\nterminal types known to be problematic and in common use on macOS, rather\nthan sledge-hammer matching all \"xterm\" since some old \"xterm\" variants may\nlegitimately not support ANSI capabilities (suggested by brian[2]).\n\nFix incorrect line number computation when swallowing here-doc bodies. v1\nincorrectly incremented the line number by two regardless of the here-doc\nbody's actual size. I very much would like to add tests to catch this sort\nof problem, however, the current chainlint self-test framework validates\nchainlint behavior in relation only to test bodies, whereas this problem\nmanifested when a here-doc was present at the script level outside of any\ntest. It may be possible to expand the self-test framework in the future to\nallow such testing, but that may be some time off, and needn't hold up this\nseries.\n\nFOOTNOTES\n\n[1]\nhttps://lore.kernel.org/git/pull.1413.git.1668013114.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/Y2xkpJj4jLqfsggL@tapette.crustytoothpaste.net/\n\nEric Sunshine (3):\n  chainlint: sidestep impoverished macOS \"terminfo\"\n  chainlint: latch line numbers at which each token starts and ends\n  chainlint: prefix annotated test definition with line numbers\n\n t/Makefile     |  2 +-\n t/chainlint.pl | 70 ++++++++++++++++++++++++++++++++++----------------\n 2 files changed, 49 insertions(+), 23 deletions(-)\n\n\nbase-commit: 73c768dae9ea4838736693965b25ba34e941ac88\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1413%2Fsunshineco%2Fchainlintline-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1413/sunshineco/chainlintline-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1413\n\nRange-diff vs v1:\n\n 1:  b85b28e5a6b ! 1:  de482cf9cf1 chainlint: sidestep impoverished macOS \"terminfo\"\n     @@ Commit message\n          chainlint: sidestep impoverished macOS \"terminfo\"\n      \n          Although the macOS Terminal.app is \"xterm\"-compatible, its corresponding\n     -    \"terminfo\" entry neglects to mention capabilities which Terminal.app\n     +    \"terminfo\" entries -- such as \"xterm\", \"xterm-256color\", and\n     +    \"xterm-new\"[1] -- neglect to mention capabilities which Terminal.app\n          actually supports (such as \"dim text\"). This oversight on Apple's part\n          ends up penalizing users of \"good citizen\" console programs which\n          consult \"terminfo\" to tailor their output based upon reported terminal\n          capabilities (as opposed to programs which assume that the terminal\n     -    supports ANSI codes).\n     +    supports ANSI codes). The same problem is present in other Apple\n     +    \"terminfo\" entries, such as \"nsterm\"[2], with which macOS Terminal.app\n     +    may be configured.\n      \n          Sidestep this Apple problem by imbuing get_colors() with specific\n     -    knowledge of \"xterm\" capabilities rather than trusting \"terminfo\" to\n     -    report them correctly. Although hard-coding such knowledge is ugly,\n     -    \"xterm\" support is nearly ubiquitous these days, and Git itself sets\n     -    precedence by assuming support for ANSI color codes. For non-\"xterm\",\n     -    fall back to querying \"terminfo\" via `tput` as usual.\n     +    knowledge of capabilities common to \"xterm\" and \"nsterm\", rather than\n     +    trusting \"terminfo\" to report them correctly. Although hard-coding such\n     +    knowledge is ugly, \"xterm\" support is nearly ubiquitous these days, and\n     +    Git itself sets precedence by assuming support for ANSI color codes. For\n     +    other terminal types, fall back to querying \"terminfo\" via `tput` as\n     +    usual.\n     +\n     +    FOOTNOTES\n     +\n     +    [1] iTerm2 FAQ suggests \"xterm-new\": https://iterm2.com/faq.html\n     +\n     +    [2] Neovim documentation recommends terminal type \"nsterm\" with\n     +        Terminal.app: https://neovim.io/doc/user/term.html#terminfo\n      \n          Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n      \n     @@ t/chainlint.pl: my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '',\n      -\t\t   green => `tput setaf 2`,\n      -\t\t   red   => `tput setaf 1`);\n      -\tchomp(%COLORS);\n     -+\tif ($ENV{TERM} =~ /\\bxterm\\b/) {\n     ++\tif ($ENV{TERM} =~ /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/) {\n      +\t\t%COLORS = (bold  => \"\\e[1m\",\n      +\t\t\t   rev   => \"\\e[7m\",\n      +\t\t\t   reset => \"\\e[0m\",\n 2:  c8a316426be ! 2:  84ddc6707fb chainlint: latch line numbers at which each token starts and ends\n     @@ t/chainlint.pl: sub scan_balanced {\n       }\n       \n      @@ t/chainlint.pl: sub swallow_heredocs {\n     + \tmy $b = $self->{buff};\n     + \tmy $tags = $self->{heretags};\n       \twhile (my $tag = shift @$tags) {\n     ++\t\tmy $start = pos($$b);\n       \t\tmy $indent = $tag =~ s/^\\t// ? '\\\\s*' : '';\n       \t\t$$b =~ /(?:\\G|\\n)$indent\\Q$tag\\E(?:\\n|\\z)/gc;\n     -+\t\tmy $body = $&;\n     ++\t\tmy $body = substr($$b, $start, pos($$b) - $start);\n      +\t\t$self->{lineno} += () = $body =~ /\\n/sg;\n       \t}\n       }\n 3:  380b146abd1 ! 3:  3cb4ff4d330 chainlint: prefix annotated test definition with line numbers\n     @@ t/chainlint.pl: if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) {\n       sub get_colors {\n       \treturn \\%COLORS if %COLORS;\n      @@ t/chainlint.pl: sub get_colors {\n     - \tif ($ENV{TERM} =~ /\\bxterm\\b/) {\n     + \tif ($ENV{TERM} =~ /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/) {\n       \t\t%COLORS = (bold  => \"\\e[1m\",\n       \t\t\t   rev   => \"\\e[7m\",\n      +\t\t\t   dim   => \"\\e[2m\",\n\n-- \ngitgitgadget\n"},{"id":"467131","messageId":"de482cf9cf1c791418e4279523123580f330245b.1668152094.git.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.v2.git.1668152094.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T07:34:52Z","receivedAt":"2022-11-11T07:35:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nAlthough the macOS Terminal.app is \"xterm\"-compatible, its corresponding\n\"terminfo\" entries -- such as \"xterm\", \"xterm-256color\", and\n\"xterm-new\"[1] -- neglect to mention capabilities which Terminal.app\nactually supports (such as \"dim text\"). This oversight on Apple's part\nends up penalizing users of \"good citizen\" console programs which\nconsult \"terminfo\" to tailor their output based upon reported terminal\ncapabilities (as opposed to programs which assume that the terminal\nsupports ANSI codes). The same problem is present in other Apple\n\"terminfo\" entries, such as \"nsterm\"[2], with which macOS Terminal.app\nmay be configured.\n\nSidestep this Apple problem by imbuing get_colors() with specific\nknowledge of capabilities common to \"xterm\" and \"nsterm\", rather than\ntrusting \"terminfo\" to report them correctly. Although hard-coding such\nknowledge is ugly, \"xterm\" support is nearly ubiquitous these days, and\nGit itself sets precedence by assuming support for ANSI color codes. For\nother terminal types, fall back to querying \"terminfo\" via `tput` as\nusual.\n\nFOOTNOTES\n\n[1] iTerm2 FAQ suggests \"xterm-new\": https://iterm2.com/faq.html\n\n[2] Neovim documentation recommends terminal type \"nsterm\" with\n    Terminal.app: https://neovim.io/doc/user/term.html#terminfo\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/chainlint.pl | 35 +++++++++++++++++++++++------------\n 1 file changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/t/chainlint.pl b/t/chainlint.pl\nindex 7972c5bbe6f..0ee5cc36437 100755\n--- a/t/chainlint.pl\n+++ b/t/chainlint.pl\n@@ -653,21 +653,32 @@ my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '', green => '', red\n my %COLORS = ();\n sub get_colors {\n \treturn \\%COLORS if %COLORS;\n-\tif (exists($ENV{NO_COLOR}) ||\n-\t    system(\"tput sgr0 >/dev/null 2>&1\") != 0 ||\n-\t    system(\"tput bold >/dev/null 2>&1\") != 0 ||\n-\t    system(\"tput rev  >/dev/null 2>&1\") != 0 ||\n-\t    system(\"tput setaf 1 >/dev/null 2>&1\") != 0) {\n+\tif (exists($ENV{NO_COLOR})) {\n \t\t%COLORS = @NOCOLORS;\n \t\treturn \\%COLORS;\n \t}\n-\t%COLORS = (bold  => `tput bold`,\n-\t\t   rev   => `tput rev`,\n-\t\t   reset => `tput sgr0`,\n-\t\t   blue  => `tput setaf 4`,\n-\t\t   green => `tput setaf 2`,\n-\t\t   red   => `tput setaf 1`);\n-\tchomp(%COLORS);\n+\tif ($ENV{TERM} =~ /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/) {\n+\t\t%COLORS = (bold  => \"\\e[1m\",\n+\t\t\t   rev   => \"\\e[7m\",\n+\t\t\t   reset => \"\\e[0m\",\n+\t\t\t   blue  => \"\\e[34m\",\n+\t\t\t   green => \"\\e[32m\",\n+\t\t\t   red   => \"\\e[31m\");\n+\t\treturn \\%COLORS;\n+\t}\n+\tif (system(\"tput sgr0 >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput bold >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput rev  >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput setaf 1 >/dev/null 2>&1\") == 0) {\n+\t\t%COLORS = (bold  => `tput bold`,\n+\t\t\t   rev   => `tput rev`,\n+\t\t\t   reset => `tput sgr0`,\n+\t\t\t   blue  => `tput setaf 4`,\n+\t\t\t   green => `tput setaf 2`,\n+\t\t\t   red   => `tput setaf 1`);\n+\t\treturn \\%COLORS;\n+\t}\n+\t%COLORS = @NOCOLORS;\n \treturn \\%COLORS;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"467132","messageId":"84ddc6707fb0fd5e1be675ba587e453c55a76acc.1668152094.git.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.v2.git.1668152094.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] chainlint: latch line numbers at which each token starts and ends","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T07:34:53Z","receivedAt":"2022-11-11T07:35:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nWhen chainlint detects problems in a test, it prints out the name of the\ntest script, the name of the problematic test, and a copy of the test\ndefinition with \"?!FOO?!\" annotations inserted at the locations where\nproblems were detected. Taken together this information is sufficient\nfor the test author to identify the problematic code in the original\ntest definition. However, in a lengthy script or a lengthy test\ndefinition, the author may still end up using the editor's search\nfeature to home in on the exact problem location.\n\nTo further assist the test author, an upcoming change will display line\nnumbers along with the annotated test definition, thus allowing the\nauthor to jump directly to each problematic line. As preparation,\nupgrade Lexer to latch the line numbers at which each token starts and\nends, and return that information with the token itself.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/chainlint.pl | 25 +++++++++++++++++--------\n 1 file changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/t/chainlint.pl b/t/chainlint.pl\nindex 0ee5cc36437..67c2c5ebee8 100755\n--- a/t/chainlint.pl\n+++ b/t/chainlint.pl\n@@ -67,6 +67,7 @@ sub new {\n \tbless {\n \t\tparser => $parser,\n \t\tbuff => $s,\n+\t\tlineno => 1,\n \t\theretags => []\n \t} => $class;\n }\n@@ -97,7 +98,9 @@ sub scan_op {\n sub scan_sqstring {\n \tmy $self = shift @_;\n \t${$self->{buff}} =~ /\\G([^']*'|.*\\z)/sgc;\n-\treturn \"'\" . $1;\n+\tmy $s = $1;\n+\t$self->{lineno} += () = $s =~ /\\n/sg;\n+\treturn \"'\" . $s;\n }\n \n sub scan_dqstring {\n@@ -115,7 +118,7 @@ sub scan_dqstring {\n \t\tif ($c eq '\\\\') {\n \t\t\t$s .= '\\\\', last unless $$b =~ /\\G(.)/sgc;\n \t\t\t$c = $1;\n-\t\t\tnext if $c eq \"\\n\"; # line splice\n+\t\t\t$self->{lineno}++, next if $c eq \"\\n\"; # line splice\n \t\t\t# backslash escapes only $, `, \", \\ in dq-string\n \t\t\t$s .= '\\\\' unless $c =~ /^[\\$`\"\\\\]$/;\n \t\t\t$s .= $c;\n@@ -123,6 +126,7 @@ sub scan_dqstring {\n \t\t}\n \t\tdie(\"internal error scanning dq-string '$c'\\n\");\n \t}\n+\t$self->{lineno} += () = $s =~ /\\n/sg;\n \treturn $s;\n }\n \n@@ -137,6 +141,7 @@ sub scan_balanced {\n \t\t$depth--;\n \t\tlast if $depth == 0;\n \t}\n+\t$self->{lineno} += () = $s =~ /\\n/sg;\n \treturn $s;\n }\n \n@@ -163,8 +168,11 @@ sub swallow_heredocs {\n \tmy $b = $self->{buff};\n \tmy $tags = $self->{heretags};\n \twhile (my $tag = shift @$tags) {\n+\t\tmy $start = pos($$b);\n \t\tmy $indent = $tag =~ s/^\\t// ? '\\\\s*' : '';\n \t\t$$b =~ /(?:\\G|\\n)$indent\\Q$tag\\E(?:\\n|\\z)/gc;\n+\t\tmy $body = substr($$b, $start, pos($$b) - $start);\n+\t\t$self->{lineno} += () = $body =~ /\\n/sg;\n \t}\n }\n \n@@ -172,11 +180,12 @@ sub scan_token {\n \tmy $self = shift @_;\n \tmy $b = $self->{buff};\n \tmy $token = '';\n-\tmy $start;\n+\tmy ($start, $startln);\n RESTART:\n+\t$startln = $self->{lineno};\n \t$$b =~ /\\G[ \\t]+/gc; # skip whitespace (but not newline)\n \t$start = pos($$b) || 0;\n-\treturn [\"\\n\", $start, pos($$b)] if $$b =~ /\\G#[^\\n]*(?:\\n|\\z)/gc; # comment\n+\t$self->{lineno}++, return [\"\\n\", $start, pos($$b), $startln, $startln] if $$b =~ /\\G#[^\\n]*(?:\\n|\\z)/gc; # comment\n \twhile (1) {\n \t\t# slurp up non-special characters\n \t\t$token .= $1 if $$b =~ /\\G([^\\\\;&|<>(){}'\"\\$\\s]+)/gc;\n@@ -188,20 +197,20 @@ RESTART:\n \t\t$token .= $self->scan_sqstring(), next if $c eq \"'\";\n \t\t$token .= $self->scan_dqstring(), next if $c eq '\"';\n \t\t$token .= $c . $self->scan_dollar(), next if $c eq '$';\n-\t\t$self->swallow_heredocs(), $token = $c, last if $c eq \"\\n\";\n+\t\t$self->{lineno}++, $self->swallow_heredocs(), $token = $c, last if $c eq \"\\n\";\n \t\t$token = $self->scan_op($c), last if $c =~ /^[;&|<>]$/;\n \t\t$token = $c, last if $c =~ /^[(){}]$/;\n \t\tif ($c eq '\\\\') {\n \t\t\t$token .= '\\\\', last unless $$b =~ /\\G(.)/sgc;\n \t\t\t$c = $1;\n-\t\t\tnext if $c eq \"\\n\" && length($token); # line splice\n-\t\t\tgoto RESTART if $c eq \"\\n\"; # line splice\n+\t\t\t$self->{lineno}++, next if $c eq \"\\n\" && length($token); # line splice\n+\t\t\t$self->{lineno}++, goto RESTART if $c eq \"\\n\"; # line splice\n \t\t\t$token .= '\\\\' . $c;\n \t\t\tnext;\n \t\t}\n \t\tdie(\"internal error scanning character '$c'\\n\");\n \t}\n-\treturn length($token) ? [$token, $start, pos($$b)] : undef;\n+\treturn length($token) ? [$token, $start, pos($$b), $startln, $self->{lineno}] : undef;\n }\n \n # ShellParser parses POSIX shell scripts (with minor extensions for Bash). It\n-- \ngitgitgadget\n\n"},{"id":"467133","messageId":"3cb4ff4d330acf0f6feaa53c499d1931cc793dc6.1668152094.git.gitgitgadget@gmail.com","threadId":"58777","inReplyTo":"pull.1413.v2.git.1668152094.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] chainlint: prefix annotated test definition with line numbers","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-11T07:34:54Z","receivedAt":"2022-11-11T07:35:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nWhen chainlint detects problems in a test, it prints out the name of the\ntest script, the name of the problematic test, and a copy of the test\ndefinition with \"?!FOO?!\" annotations inserted at the locations where\nproblems were detected. Taken together this information is sufficient\nfor the test author to identify the problematic code in the original\ntest definition. However, in a lengthy script or a lengthy test\ndefinition, the author may still end up using the editor's search\nfeature to home in on the exact problem location.\n\nTo further assist the test author, display line numbers along with the\nannotated test definition, thus allowing the author to jump directly to\neach problematic line.\n\nSuggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/Makefile     |  2 +-\n t/chainlint.pl | 10 ++++++++--\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 882782a519c..2c2b2522402 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -94,7 +94,7 @@ check-chainlint:\n \t\tdone \\\n \t} >'$(CHAINLINTTMP_SQ)'/expect && \\\n \t$(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \\\n-\t\tgrep -v '^[ \t]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \\\n+\t\tsed -e 's/^[1-9][0-9]* //;/^[ \t]*$$/d' >'$(CHAINLINTTMP_SQ)'/actual && \\\n \tif test -f ../GIT-BUILD-OPTIONS; then \\\n \t\t. ../GIT-BUILD-OPTIONS; \\\n \tfi && \\\ndiff --git a/t/chainlint.pl b/t/chainlint.pl\nindex 67c2c5ebee8..4e47e808d01 100755\n--- a/t/chainlint.pl\n+++ b/t/chainlint.pl\n@@ -614,6 +614,7 @@ sub check_test {\n \tmy $problems = $parser->{problems};\n \treturn unless $emit_all || @$problems;\n \tmy $c = main::fd_colors(1);\n+\tmy $lineno = $_[1]->[3];\n \tmy $start = 0;\n \tmy $checked = '';\n \tfor (sort {$a->[1]->[2] <=> $b->[1]->[2]} @$problems) {\n@@ -623,10 +624,12 @@ sub check_test {\n \t\t$start = $pos;\n \t}\n \t$checked .= substr($body, $start);\n-\t$checked =~ s/^\\n//;\n+\t$checked =~ s/^/$lineno++ . ' '/mge;\n+\t$checked =~ s/^\\d+ \\n//;\n \t$checked =~ s/(\\s) \\?!/$1?!/mg;\n \t$checked =~ s/\\?! (\\s)/?!$1/mg;\n \t$checked =~ s/(\\?![^?]+\\?!)/$c->{rev}$c->{red}$1$c->{reset}/mg;\n+\t$checked =~ s/^\\d+/$c->{dim}$&$c->{reset}/mg;\n \t$checked .= \"\\n\" unless $checked =~ /\\n$/;\n \tpush(@{$self->{output}}, \"$c->{blue}# chainlint: $title$c->{reset}\\n$checked\");\n }\n@@ -658,7 +661,7 @@ if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) {\n # thread and ignore %ENV changes in subthreads.\n $ENV{TERM} = $ENV{USER_TERM} if $ENV{USER_TERM};\n \n-my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '', green => '', red => '');\n+my @NOCOLORS = (bold => '', rev => '', dim => '', reset => '', blue => '', green => '', red => '');\n my %COLORS = ();\n sub get_colors {\n \treturn \\%COLORS if %COLORS;\n@@ -669,6 +672,7 @@ sub get_colors {\n \tif ($ENV{TERM} =~ /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/) {\n \t\t%COLORS = (bold  => \"\\e[1m\",\n \t\t\t   rev   => \"\\e[7m\",\n+\t\t\t   dim   => \"\\e[2m\",\n \t\t\t   reset => \"\\e[0m\",\n \t\t\t   blue  => \"\\e[34m\",\n \t\t\t   green => \"\\e[32m\",\n@@ -678,9 +682,11 @@ sub get_colors {\n \tif (system(\"tput sgr0 >/dev/null 2>&1\") == 0 &&\n \t    system(\"tput bold >/dev/null 2>&1\") == 0 &&\n \t    system(\"tput rev  >/dev/null 2>&1\") == 0 &&\n+\t    system(\"tput dim  >/dev/null 2>&1\") == 0 &&\n \t    system(\"tput setaf 1 >/dev/null 2>&1\") == 0) {\n \t\t%COLORS = (bold  => `tput bold`,\n \t\t\t   rev   => `tput rev`,\n+\t\t\t   dim   => `tput dim`,\n \t\t\t   reset => `tput sgr0`,\n \t\t\t   blue  => `tput setaf 4`,\n \t\t\t   green => `tput setaf 2`,\n-- \ngitgitgadget\n"},{"id":"467141","messageId":"221111.865yflo7p7.gmgdl@evledraar.gmail.com","threadId":"58777","inReplyTo":"de482cf9cf1c791418e4279523123580f330245b.1668152094.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-11T14:55:15Z","receivedAt":"2022-11-11T15:07:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 11 2022, Eric Sunshine via GitGitGadget wrote:\n\n> From: Eric Sunshine <sunshine@sunshineco.com>\n>\n> Although the macOS Terminal.app is \"xterm\"-compatible, its corresponding\n> \"terminfo\" entries -- such as \"xterm\", \"xterm-256color\", and\n> \"xterm-new\"[1] -- neglect to mention capabilities which Terminal.app\n> actually supports (such as \"dim text\"). This oversight on Apple's part\n> ends up penalizing users of \"good citizen\" console programs which\n> consult \"terminfo\" to tailor their output based upon reported terminal\n> capabilities (as opposed to programs which assume that the terminal\n> supports ANSI codes). The same problem is present in other Apple\n> \"terminfo\" entries, such as \"nsterm\"[2], with which macOS Terminal.app\n> may be configured.\n>\n> Sidestep this Apple problem by imbuing get_colors() with specific\n> knowledge of capabilities common to \"xterm\" and \"nsterm\", rather than\n> trusting \"terminfo\" to report them correctly. Although hard-coding such\n> knowledge is ugly, \"xterm\" support is nearly ubiquitous these days, and\n> Git itself sets precedence by assuming support for ANSI color codes. For\n> other terminal types, fall back to querying \"terminfo\" via `tput` as\n> usual.\n>\n> FOOTNOTES\n>\n> [1] iTerm2 FAQ suggests \"xterm-new\": https://iterm2.com/faq.html\n>\n> [2] Neovim documentation recommends terminal type \"nsterm\" with\n>     Terminal.app: https://neovim.io/doc/user/term.html#terminfo\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  t/chainlint.pl | 35 +++++++++++++++++++++++------------\n>  1 file changed, 23 insertions(+), 12 deletions(-)\n>\n> diff --git a/t/chainlint.pl b/t/chainlint.pl\n> index 7972c5bbe6f..0ee5cc36437 100755\n> --- a/t/chainlint.pl\n> +++ b/t/chainlint.pl\n> @@ -653,21 +653,32 @@ my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '', green => '', red\n>  my %COLORS = ();\n>  sub get_colors {\n>  \treturn \\%COLORS if %COLORS;\n> -\tif (exists($ENV{NO_COLOR}) ||\n> -\t    system(\"tput sgr0 >/dev/null 2>&1\") != 0 ||\n> -\t    system(\"tput bold >/dev/null 2>&1\") != 0 ||\n> -\t    system(\"tput rev  >/dev/null 2>&1\") != 0 ||\n> -\t    system(\"tput setaf 1 >/dev/null 2>&1\") != 0) {\n> +\tif (exists($ENV{NO_COLOR})) {\n>  \t\t%COLORS = @NOCOLORS;\n>  \t\treturn \\%COLORS;\n>  \t}\n> -\t%COLORS = (bold  => `tput bold`,\n> -\t\t   rev   => `tput rev`,\n> -\t\t   reset => `tput sgr0`,\n> -\t\t   blue  => `tput setaf 4`,\n> -\t\t   green => `tput setaf 2`,\n> -\t\t   red   => `tput setaf 1`);\n> -\tchomp(%COLORS);\n> +\tif ($ENV{TERM} =~ /xterm|xterm-\\d+color|xterm-new|xterm-direct|nsterm|nsterm-\\d+color|nsterm-direct/) {\n> +\t\t%COLORS = (bold  => \"\\e[1m\",\n> +\t\t\t   rev   => \"\\e[7m\",\n> +\t\t\t   reset => \"\\e[0m\",\n> +\t\t\t   blue  => \"\\e[34m\",\n> +\t\t\t   green => \"\\e[32m\",\n> +\t\t\t   red   => \"\\e[31m\");\n> +\t\treturn \\%COLORS;\n> +\t}\n> +\tif (system(\"tput sgr0 >/dev/null 2>&1\") == 0 &&\n> +\t    system(\"tput bold >/dev/null 2>&1\") == 0 &&\n> +\t    system(\"tput rev  >/dev/null 2>&1\") == 0 &&\n> +\t    system(\"tput setaf 1 >/dev/null 2>&1\") == 0) {\n> +\t\t%COLORS = (bold  => `tput bold`,\n> +\t\t\t   rev   => `tput rev`,\n> +\t\t\t   reset => `tput sgr0`,\n> +\t\t\t   blue  => `tput setaf 4`,\n> +\t\t\t   green => `tput setaf 2`,\n> +\t\t\t   red   => `tput setaf 1`);\n> +\t\treturn \\%COLORS;\n> +\t}\n> +\t%COLORS = @NOCOLORS;\n>  \treturn \\%COLORS;\n>  }\n\nDoesn't test-lib.sh have the same problem then?\n\nThis is somewhat of an aside, as we're hardcoding thees colors in\nt/chainlint.pl now, but I wondered when that was added (but I don't\nthink I commented then) why it needed to be re-hardcoding the coloring\nwe've got in test-lib.sh.\n\nI.e. if test-lib.sh is running it could we handle these cases, and just\nexport a variable with the color info for \"bold\" or whatever in\nGIT_TEST_COLOR_BOLD, then pick that up?\n\nI have a local semi-related patch which made much the same change to\ntest-lib.sh itself, to support --color without going through whether\ntput thinks we support colors:\nhttps://github.com/avar/git/commit/c4914db758b\n\nI think this is fine for now if you don't want to poke more at it, but\nmaybe this should all be eventually combined?\n\nI also wonder to what extent this needs to be re-inventing\nTerm::ANSIColor, which has shipped with Perl since 5.6, so we can use it\nwithout worrying about version compat, but that's another topic...\n"},{"id":"467142","messageId":"221111.861qq9o7m4.gmgdl@evledraar.gmail.com","threadId":"58777","inReplyTo":"pull.1413.git.1668013114.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] chainlint: emit line numbers alongside test definitions","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-11T15:03:01Z","receivedAt":"2022-11-11T15:07:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 09 2022, Eric Sunshine via GitGitGadget wrote:\n\n> This is atop \"es/chainlint-output\"[2].\n>\n> (Note to self: Fortify against Ævar's nerd-snipe blacklist evasion.)\n\nMy only regret is not asking for a pony :)\n\nThis looks great, thanks. I read over the v2 (just commenting on the v1\nCL for the above comment). I left a note about a potential follow-up\nabout the color detection, but that's aside from the main change here,\nso I think it would be good to just get some version of your v2 as-is,\nunless you're super keen to spend more time fiddling with this...\n"},{"id":"467149","messageId":"CAPig+cRwDeGyniiVGqmdMePgmR6GiYQOvNP+GUeT__zpuWV1Fg@mail.gmail.com","threadId":"58777","inReplyTo":"221111.865yflo7p7.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-11-11T16:44:22Z","receivedAt":"2022-11-11T16:44:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 11, 2022 at 10:02 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Fri, Nov 11 2022, Eric Sunshine via GitGitGadget wrote:\n> > Sidestep this Apple problem by imbuing get_colors() with specific\n> > knowledge of capabilities common to \"xterm\" and \"nsterm\", rather than\n> > trusting \"terminfo\" to report them correctly. Although hard-coding such\n> > knowledge is ugly, \"xterm\" support is nearly ubiquitous these days, and\n> > Git itself sets precedence by assuming support for ANSI color codes. For\n> > other terminal types, fall back to querying \"terminfo\" via `tput` as\n> > usual.\n>\n> Doesn't test-lib.sh have the same problem then?\n\nGenerally speaking, yes, but in practice, no, not for this particular\ncase. I specifically wanted to use \"dim\" here in chainlint.pl, which\nis (oddly) missing in Apple's terminfo. test-lib.sh just uses some\ncolors, bold, reverse, and \"reset\", all of which are present in\nApple's terminfo.\n\n> This is somewhat of an aside, as we're hardcoding thees colors in\n> t/chainlint.pl now, but I wondered when that was added (but I don't\n> think I commented then) why it needed to be re-hardcoding the coloring\n> we've got in test-lib.sh.\n>\n> I.e. if test-lib.sh is running it could we handle these cases, and just\n> export a variable with the color info for \"bold\" or whatever in\n> GIT_TEST_COLOR_BOLD, then pick that up?\n\nWhen adding colorizing to chainlint.pl, one of my very first thoughts\nwas to somehow reuse the color information from test-lib.sh. That's\nrelatively easy in the case when the test script is being run\nstandalone because it has already \"sourced\" test-lib.sh before it runs\nchainlint.pl, thus could pass the color information along to\nchainlint.pl somehow. But the other case, when \"make test\" (or \"make\ntest-chainlint\", etc.) is used is harder because that's just the\nMakefile running chainlint.pl directly, so test-lib.sh isn't involved\nin the equation. It would probably be possible to make it work, but\nthe solutions seemed ugly and too invasive, especially for an initial\nimplementation of colorizing in chainlint.pl.\n\n> I have a local semi-related patch which made much the same change to\n> test-lib.sh itself, to support --color without going through whether\n> tput thinks we support colors:\n> https://github.com/avar/git/commit/c4914db758b\n>\n> I think this is fine for now if you don't want to poke more at it, but\n> maybe this should all be eventually combined?\n\nPeff also expressed such a sentiment[1][2][3]. I'm still somewhat\nhesitant to make chainlint.pl dependent upon so much outside machinery\nsince it's still nicely standalone and _might_ be useful elsewhere.\n\n> I also wonder to what extent this needs to be re-inventing\n> Term::ANSIColor, which has shipped with Perl since 5.6, so we can use it\n> without worrying about version compat, but that's another topic...\n\nGah, why didn't I know about this sooner?! (Note to self: hit self\nover head.) Back when I was adding colorizing to chainlint.pl, I\nsearched around to determine if Perl had any built-in colorizing\nsupport, but I completely overlooked this. I must have missed it\nbecause I was focusing on \"curses\" since I thought I would need to\npull color codes directly from \"curses\", and Perl doesn't have a\nstandard (shipped) \"curses\" module. I even said as much to Peff[4].\n\nSince it's been shipping with Perl for quite some time,\nTerm::ANSIColor would be a much nicer solution; worth looking into.\n\n[1]: https://lore.kernel.org/git/Yx%2FLpUglpjY5ZNas@coredump.intra.peff.net/\n[2]: https://lore.kernel.org/git/Yx%2FeG5xJonNh7Dsz@coredump.intra.peff.net/\n[3]: https://lore.kernel.org/git/Yx%2FPnWnkYAuWToiz@coredump.intra.peff.net/\n[4]: https://lore.kernel.org/git/CAPig+cRJVn-mbA6-jOmNfDJtK_nX4ZTw+OcNShvvz8zcQYbCHQ@mail.gmail.com/\n"},{"id":"467150","messageId":"CAPig+cRCSg=iVLUmLG=W47ofojU56CcFsobNZK5z5h9LdzXs0Q@mail.gmail.com","threadId":"58777","inReplyTo":"CAPig+cRwDeGyniiVGqmdMePgmR6GiYQOvNP+GUeT__zpuWV1Fg@mail.gmail.com","subject":"Re: [PATCH v2 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-11-11T17:15:12Z","receivedAt":"2022-11-11T17:15:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Nov 11, 2022 at 11:44 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Fri, Nov 11, 2022 at 10:02 AM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n> > I also wonder to what extent this needs to be re-inventing\n> > Term::ANSIColor, which has shipped with Perl since 5.6, so we can use it\n> > without worrying about version compat, but that's another topic...\n>\n> Gah, why didn't I know about this sooner?! [...]\n>\n> Since it's been shipping with Perl for quite some time,\n> Term::ANSIColor would be a much nicer solution; worth looking into.\n\nIn retrospect, I may have looked at Term::ANSIColor at the time but\ndecided to avoid it since it assumes the terminal understands ANSI\ncodes, and I was looking for a more general solution which respected\nthe terminal's capabilities as reported by \"terminfo\".\n\nAnd, reading up on it now, I'm not finding much benefit to\nTerm::ANSIColor over what is already implemented in chainlint.pl.\nParticularly disheartening is that (as far as I can tell)\nTerm::ANSIColor doesn't provide a way to interrogate whether or not it\nis suitable to use ANSI codes with the terminal in question, but\ninstead makes a blanket assumption that the terminal supports ANSI\ncodes unconditionally.\n\nSo, I think the fixed-up colorizing as implemented by v2 of this patch\nseries is good enough for now. It can always be revisited later if\nsomething warrants it.\n"},{"id":"467163","messageId":"Y27FK61h4awaNYJf@nand.local","threadId":"58777","inReplyTo":"CAPig+cRCSg=iVLUmLG=W47ofojU56CcFsobNZK5z5h9LdzXs0Q@mail.gmail.com","subject":"Re: [PATCH v2 1/3] chainlint: sidestep impoverished macOS \"terminfo\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-11T21:56:59Z","receivedAt":"2022-11-11T21:57:05Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Nov 11, 2022 at 12:15:12PM -0500, Eric Sunshine wrote:\n> So, I think the fixed-up colorizing as implemented by v2 of this patch\n> series is good enough for now. It can always be revisited later if\n> something warrants it.\n\nYeah, agree. Let's not let perfect be the enemy of the good ;-).\n\nThanks,\nTaylor\n"}]}