git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org,
	Sebastian Celis <sebastian@sebastiancelis.com>,
	Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH v4 7/7] t7006-pager: if stdout is not a terminal, make a new one
Date: Sun, 21 Feb 2010 02:30:59 -0500	[thread overview]
Message-ID: <20100221073058.GC2840@coredump.intra.peff.net> (raw)
In-Reply-To: <20100221020922.GB7651@progeny.tock>

On Sat, Feb 20, 2010 at 08:09:22PM -0600, Jonathan Nieder wrote:

> Testing pagination requires (fake or real) access to a terminal so we
> can see whether the pagination automatically kicks in, which makes it
> hard to get good coverage when running tests without --verbose.  There
> are a number of ways to work around that:
> 
>  - Replace all isatty calls with calls to a custom xisatty wrapper
>    that usually checks for a terminal but can be overridden for tests.
>    This would be workable, but it would require implementing xisatty
>    separately in three languages (C, shell, and perl) and making sure
>    that any code that is to be tested always uses the wrapper.
> 
>  - Redirect stdout to /dev/tty.  This would be problematic because
>    there might be no terminal available, and even if a terminal is
>    available, it might not be appropriate to spew output to it.
> 
>  - Create a new pseudo-terminal on the fly and capture its output.
> 
> This patch implements the third approach.

Just to wrap up my end of this patch discussion, I think the approach
you take here is the sanest one. While it would be nice to get test
coverage on every system, I don't think it is worth the effort of
trying to write portable terminal creation code. And this way at least
the code in git is fairly minimal.

So looks good to me.

-Peff

  reply	other threads:[~2010-02-21  7:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-19  6:50 [PATCH v2 0/7] Re: 'git svn log' no longer uses the pager Jonathan Nieder
2010-02-19  6:51 ` [PATCH 1/7] Fix 'git var' usage synopsis Jonathan Nieder
2010-02-19  7:00 ` [PATCH 2/7] Make 'git var GIT_PAGER' always print the configured pager Jonathan Nieder
2010-02-19  7:06 ` [PATCH 3/7] git.1: Clarify the behavior of the --paginate option Jonathan Nieder
2010-02-19  7:09 ` [PATCH 4/7] git svn: Fix launching of pager Jonathan Nieder
2010-02-19  7:12 ` [PATCH 5/7] am: " Jonathan Nieder
2010-02-19  7:18 ` [PATCH 6/7] tests: Add tests for automatic use " Jonathan Nieder
2010-02-20 17:33   ` Junio C Hamano
2010-02-21  2:03     ` [PATCH v2 " Jonathan Nieder
2010-02-21  2:09       ` [PATCH v4 7/7] t7006-pager: if stdout is not a terminal, make a new one Jonathan Nieder
2010-02-21  7:30         ` Jeff King [this message]
2010-02-22  8:19       ` [PATCH v2 6/7] tests: Add tests for automatic use of pager Johannes Sixt
2010-02-22  8:46         ` [PATCH 8/7] tests: Fix race condition in t7006-pager Jonathan Nieder
2010-02-22  9:12           ` Jonathan Nieder
2010-02-19  7:23 ` [PATCH/RFC 7/7] t7006-pager: if stdout is not a terminal, make a new one Jonathan Nieder
2010-02-19  8:08   ` Jeff King
2010-02-19  8:19     ` Jonathan Nieder
2010-02-19  8:34       ` Jeff King
2010-02-19 16:25         ` Brandon Casey
2010-02-20  0:29           ` Brandon Casey
2010-02-20  0:39             ` Jonathan Nieder
2010-02-20  3:42               ` Brandon Casey
2010-02-20  5:25                 ` [PATCH v2 " Jonathan Nieder
2010-02-20  6:53                   ` Junio C Hamano
2010-02-20  8:50                     ` [PATCH v3 " Jonathan Nieder
2010-02-20  9:48                       ` [PATCH squash] Simplify test-terminal.perl Jonathan Nieder

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=20100221073058.GC2840@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jrnieder@gmail.com \
    --cc=sebastian@sebastiancelis.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).