Re: [PATCH 3/7] git.c: provide setup_auto_pager()

From: Jeff King <peff@peff.net>
Date: Jul 11, 2017, 10:37 UTC
Message-ID: <20170711103715.a4jfbjop227bfuhh@sigill.intra.peff.net>
In-Reply-To: <1d639a4c7be5f1c173502f79e85aadd017ef5f7c.1499723297.git.martin.agren@gmail.com>

On Mon, Jul 10, 2017 at 11:55:16PM +0200, Martin Ågren wrote:

> +void setup_auto_pager(const char *cmd, int def)
> +{
> +	if (use_pager != -1)
> +		return;

I think you probably also want to return early here if pager_in_use() is true. If a script runs "git tag -l", you wouldn't want to start a new pager when the outer script has already been paged (either via config, or via "git -p").

Show 16 quoted lines

> +	use_pager = check_pager_config(cmd);
> +
> +	if (use_pager == -1) {
> +		struct strbuf buf = STRBUF_INIT;
> +		size_t len;
> +
> +		strbuf_addstr(&buf, cmd);
> +		len = buf.len;
> +		while (use_pager == -1 && len--) {
> +			if (buf.buf[len] == '.') {
> +				strbuf_setlen(&buf, len);
> +				use_pager = check_pager_config(buf.buf);
> +			}
> +		}
> +		strbuf_release(&buf);
> +	}

This looks good. I wondered if we could fold this all into a single loop, rather than having the extra check_pager_config() beforehand (which confused me for a half-second at first). But I tried and I don't think the result ended up any more readable.

-Peff