git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Eric Sunshine via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Taylor Blau" <me@ttaylorr.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Eric Sunshine" <sunshine@sunshineco.com>
Subject: [PATCH v2 0/3] chainlint: emit line numbers alongside test definitions
Date: Fri, 11 Nov 2022 07:34:51 +0000	[thread overview]
Message-ID: <pull.1413.v2.git.1668152094.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1413.git.1668013114.gitgitgadget@gmail.com>

This is a re-roll of "es/chainlint-lineno"[1] which makes chainlint.pl
output line numbers alongside annotated test definitions in which problems
have been discovered.

Changes since v1:

Limit sidestepping of impoverished Apple "terminfo" entries to specific
terminal types known to be problematic and in common use on macOS, rather
than sledge-hammer matching all "xterm" since some old "xterm" variants may
legitimately not support ANSI capabilities (suggested by brian[2]).

Fix incorrect line number computation when swallowing here-doc bodies. v1
incorrectly incremented the line number by two regardless of the here-doc
body's actual size. I very much would like to add tests to catch this sort
of problem, however, the current chainlint self-test framework validates
chainlint behavior in relation only to test bodies, whereas this problem
manifested when a here-doc was present at the script level outside of any
test. It may be possible to expand the self-test framework in the future to
allow such testing, but that may be some time off, and needn't hold up this
series.

FOOTNOTES

[1]
https://lore.kernel.org/git/pull.1413.git.1668013114.gitgitgadget@gmail.com/
[2]
https://lore.kernel.org/git/Y2xkpJj4jLqfsggL@tapette.crustytoothpaste.net/

Eric Sunshine (3):
  chainlint: sidestep impoverished macOS "terminfo"
  chainlint: latch line numbers at which each token starts and ends
  chainlint: prefix annotated test definition with line numbers

 t/Makefile     |  2 +-
 t/chainlint.pl | 70 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 49 insertions(+), 23 deletions(-)


base-commit: 73c768dae9ea4838736693965b25ba34e941ac88
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1413%2Fsunshineco%2Fchainlintline-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1413/sunshineco/chainlintline-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1413

Range-diff vs v1:

 1:  b85b28e5a6b ! 1:  de482cf9cf1 chainlint: sidestep impoverished macOS "terminfo"
     @@ Commit message
          chainlint: sidestep impoverished macOS "terminfo"
      
          Although the macOS Terminal.app is "xterm"-compatible, its corresponding
     -    "terminfo" entry neglects to mention capabilities which Terminal.app
     +    "terminfo" entries -- such as "xterm", "xterm-256color", and
     +    "xterm-new"[1] -- neglect to mention capabilities which Terminal.app
          actually supports (such as "dim text"). This oversight on Apple's part
          ends up penalizing users of "good citizen" console programs which
          consult "terminfo" to tailor their output based upon reported terminal
          capabilities (as opposed to programs which assume that the terminal
     -    supports ANSI codes).
     +    supports ANSI codes). The same problem is present in other Apple
     +    "terminfo" entries, such as "nsterm"[2], with which macOS Terminal.app
     +    may be configured.
      
          Sidestep this Apple problem by imbuing get_colors() with specific
     -    knowledge of "xterm" capabilities rather than trusting "terminfo" to
     -    report them correctly. Although hard-coding such knowledge is ugly,
     -    "xterm" support is nearly ubiquitous these days, and Git itself sets
     -    precedence by assuming support for ANSI color codes. For non-"xterm",
     -    fall back to querying "terminfo" via `tput` as usual.
     +    knowledge of capabilities common to "xterm" and "nsterm", rather than
     +    trusting "terminfo" to report them correctly. Although hard-coding such
     +    knowledge is ugly, "xterm" support is nearly ubiquitous these days, and
     +    Git itself sets precedence by assuming support for ANSI color codes. For
     +    other terminal types, fall back to querying "terminfo" via `tput` as
     +    usual.
     +
     +    FOOTNOTES
     +
     +    [1] iTerm2 FAQ suggests "xterm-new": https://iterm2.com/faq.html
     +
     +    [2] Neovim documentation recommends terminal type "nsterm" with
     +        Terminal.app: https://neovim.io/doc/user/term.html#terminfo
      
          Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
      
     @@ t/chainlint.pl: my @NOCOLORS = (bold => '', rev => '', reset => '', blue => '',
      -		   green => `tput setaf 2`,
      -		   red   => `tput setaf 1`);
      -	chomp(%COLORS);
     -+	if ($ENV{TERM} =~ /\bxterm\b/) {
     ++	if ($ENV{TERM} =~ /xterm|xterm-\d+color|xterm-new|xterm-direct|nsterm|nsterm-\d+color|nsterm-direct/) {
      +		%COLORS = (bold  => "\e[1m",
      +			   rev   => "\e[7m",
      +			   reset => "\e[0m",
 2:  c8a316426be ! 2:  84ddc6707fb chainlint: latch line numbers at which each token starts and ends
     @@ t/chainlint.pl: sub scan_balanced {
       }
       
      @@ t/chainlint.pl: sub swallow_heredocs {
     + 	my $b = $self->{buff};
     + 	my $tags = $self->{heretags};
       	while (my $tag = shift @$tags) {
     ++		my $start = pos($$b);
       		my $indent = $tag =~ s/^\t// ? '\\s*' : '';
       		$$b =~ /(?:\G|\n)$indent\Q$tag\E(?:\n|\z)/gc;
     -+		my $body = $&;
     ++		my $body = substr($$b, $start, pos($$b) - $start);
      +		$self->{lineno} += () = $body =~ /\n/sg;
       	}
       }
 3:  380b146abd1 ! 3:  3cb4ff4d330 chainlint: prefix annotated test definition with line numbers
     @@ t/chainlint.pl: if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) {
       sub get_colors {
       	return \%COLORS if %COLORS;
      @@ t/chainlint.pl: sub get_colors {
     - 	if ($ENV{TERM} =~ /\bxterm\b/) {
     + 	if ($ENV{TERM} =~ /xterm|xterm-\d+color|xterm-new|xterm-direct|nsterm|nsterm-\d+color|nsterm-direct/) {
       		%COLORS = (bold  => "\e[1m",
       			   rev   => "\e[7m",
      +			   dim   => "\e[2m",

-- 
gitgitgadget

  parent reply	other threads:[~2022-11-11  7:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09 16:58 [PATCH 0/3] chainlint: emit line numbers alongside test definitions Eric Sunshine via GitGitGadget
2022-11-09 16:58 ` [PATCH 1/3] chainlint: sidestep impoverished macOS "terminfo" Eric Sunshine via GitGitGadget
2022-11-09 22:18   ` Taylor Blau
2022-11-10  2:40   ` brian m. carlson
2022-11-10  3:37     ` Eric Sunshine
2022-11-10 22:21       ` brian m. carlson
2022-11-10 22:36         ` Eric Sunshine
2022-11-10 22:48           ` brian m. carlson
2022-11-09 16:58 ` [PATCH 2/3] chainlint: latch line numbers at which each token starts and ends Eric Sunshine via GitGitGadget
2022-11-09 16:58 ` [PATCH 3/3] chainlint: prefix annotated test definition with line numbers Eric Sunshine via GitGitGadget
2022-11-09 22:22 ` [PATCH 0/3] chainlint: emit line numbers alongside test definitions Taylor Blau
2022-11-11  7:34 ` Eric Sunshine via GitGitGadget [this message]
2022-11-11  7:34   ` [PATCH v2 1/3] chainlint: sidestep impoverished macOS "terminfo" Eric Sunshine via GitGitGadget
2022-11-11 14:55     ` Ævar Arnfjörð Bjarmason
2022-11-11 16:44       ` Eric Sunshine
2022-11-11 17:15         ` Eric Sunshine
2022-11-11 21:56           ` Taylor Blau
2022-11-11  7:34   ` [PATCH v2 2/3] chainlint: latch line numbers at which each token starts and ends Eric Sunshine via GitGitGadget
2022-11-11  7:34   ` [PATCH v2 3/3] chainlint: prefix annotated test definition with line numbers Eric Sunshine via GitGitGadget
2022-11-11 15:03 ` [PATCH 0/3] chainlint: emit line numbers alongside test definitions Ævar Arnfjörð Bjarmason

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=pull.1413.v2.git.1668152094.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    /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).