git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kevin Daudt <me@ikke.info>
Cc: git@vger.kernel.org, "Rafael Ascensão" <rafa.almas@gmail.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH v2] column: show auto columns when pager is active
Date: Tue, 17 Oct 2017 12:19:20 +0900	[thread overview]
Message-ID: <xmqqtvyypikn.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171016183511.12528-1-me@ikke.info> (Kevin Daudt's message of "Mon, 16 Oct 2017 20:35:11 +0200")

Kevin Daudt <me@ikke.info> writes:

> When columns are set to automatic for git tag and the output is
> paginated by git, the output is a single column instead of multiple
> columns.
>
> Standard behaviour in git is to honor auto values when the pager is
> active, which happens for example with commands like git log showing
> colors when being paged.
>
> Since ff1e72483 (tag: change default of `pager.tag` to "on",
> 2017-08-02), the pager has been enabled by default, exposing this
> problem to more people.
>
> finalize_colopts in column.c only checks whether the output is a TTY to
> determine if columns should be enabled with columns set to auto. Also
> check if the pager is active.
>
> Adding a test for git column is possible but requires some care to work
> around a race on stdin. See commit 18d8c2693 (test_terminal: redirect
> child process' stdin to a pty, 2015-08-04). Test git tag instead, since
> that does not involve stdin, and since that was the original motivation
> for this patch.

Nicely done.

> +test_expect_success TTY 'git tag with auto-columns ' '
> +	test_commit one &&
> +	test_commit two &&
> +	test_commit three &&
> +	test_commit four &&
> +	test_commit five &&
> +	cat >expected <<\EOF &&
> +initial  one      two      three    four     five
> +EOF
> +	test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \
> +		git -c column.ui=auto tag --sort=authordate &&
> +	test_cmp expected actual.tag
> +'

I'd use <<-\EOF so that here document can be intended like other
tests, and also use expect vs actual that are used in the other
tests in the same script, instead of suddenly becoming creative
in only this single test.  I can do these clean-ups locally when
queuing so no need to resend only to collect these.

Thanks, will queue.



  reply	other threads:[~2017-10-17  3:19 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-09 21:45 [RFC] column: show auto columns when pager is active Kevin Daudt
2017-10-09 23:27 ` Eric Sunshine
2017-10-10 10:30 ` Martin Ågren
2017-10-10 14:01   ` Jeff King
2017-10-10 17:02     ` Martin Ågren
2017-10-11  4:54       ` Kevin Daudt
2017-10-12 14:24         ` Jeff King
2017-10-10 14:10 ` Jeff King
2017-10-10 14:29   ` Jeff King
2017-10-10 17:04     ` Martin Ågren
2017-10-10 18:32       ` Kevin Daudt
2017-10-11 17:23 ` [PATCH] " Kevin Daudt
2017-10-11 18:12   ` Martin Ågren
2017-10-11 18:36     ` Kevin Daudt
2017-10-11 19:02       ` Martin Ågren
2017-10-16 18:35   ` [PATCH v2] " Kevin Daudt
2017-10-17  3:19     ` Junio C Hamano [this message]
2017-10-23 21:52     ` Jonathan Nieder
2017-10-24  1:11       ` Junio C Hamano
2017-10-24  1:18         ` Jonathan Nieder
2017-10-24  6:58       ` Kevin Daudt

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqtvyypikn.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=martin.agren@gmail.com \
    --cc=me@ikke.info \
    --cc=pclouds@gmail.com \
    --cc=rafa.almas@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).