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

[Jeff King <peff@peff.net>]

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").

> +	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