Re: log --pretty/--oneline: ignore log.decorate

From: Jeff King <peff@peff.net>
Date: Apr 8, 2010, 07:30 UTC
Message-ID: <20100408073014.GA15474@coredump.intra.peff.net>
In-Reply-To: <7vmxxggsl4.fsf@alter.siamese.dyndns.org>

On Tue, Apr 06, 2010 at 02:48:55PM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > [Stalled]
> >
> > * sd/log-decorate (2010-02-17) 3 commits
> >   (merged to 'next' on 2010-03-08 at 58a6fba)
> >  + log.decorate: usability fixes
> >  + Add `log.decorate' configuration variable.
> >  + git_config_maybe_bool()
> >
> > Needs squelching the configuration setting when "--pretty=raw" is given,
> > at least, or possibly when any "--pretty" is explicitly given.
> 
> This is necessary if we want to let users specify log.decorate and still
> use gitk.  A patch should look like this (of course untested).

Hmm. You took the "any --pretty" option with this patch.

And given the markups to the test suite:

Show 7 quoted lines

> -	git log --oneline >expect.none &&
> -	git log --oneline --decorate >expect.short &&
> -	git log --oneline --decorate=full >expect.full &&
> +	git log >expect.none &&
> +	git log --decorate >expect.short &&
> +	git log --decorate=full >expect.full &&
> +	git log --oneline >expect.oneline &&

I suspect that will annoy users who have set log.decorate. Wouldn't they expect to see it with "--oneline" if they have it configured (I would guess that "git log" and "git log --oneline" are the most commonly viewed forms by humans).

It seems reasonable to say "git log --pretty=raw should be consumable by scripts", but I don't know that we need to do so for every pretty format.

However:

  $ git grep oneline *.sh
  git-rebase--interactive.sh:             git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
  git-stash.sh:           head=$(git log --no-color --abbrev-commit --pretty=oneline -n 1 HEAD --)
  git-submodule.sh:                       git log --pretty=oneline --first-parent $range | wc -l

rebase--interactive properly uses rev-list. The submodule invocation would uselessly look up the decoration, and the stash one would add it to the stash commit message. But I am inclined to say that both of those scripts are at fault, and should be converted to rev-list.

-Peff