Re: [PATCH v2 2/3] pretty: make %H/%h dependent on --abbrev[-commit]

From: Jeff King <peff@peff.net>
Date: Apr 26, 2010, 03:45 UTC
Message-ID: <20100426034520.GB16500@coredump.intra.peff.net>
In-Reply-To: <20100426034135.GB30544@progeny.tock>

On Sun, Apr 25, 2010 at 10:41:35PM -0500, Jonathan Nieder wrote:

Show 32 quoted lines

> Jonathan Nieder wrote:
> > Jeff King wrote:
> >> On Sun, Apr 25, 2010 at 10:11:37PM -0500, Jonathan Nieder wrote:
> 
> >>> +static void abbreviate_commit_hashes(char *fmt)
> >>> +{
> >>> +	char *p;
> >>> +	for (p = fmt; p != NULL; p = strchr(p + 1, '%')) {
> >>> +		p++;
> >>> +		switch (*p) {
> >>> +		case 'H':
> >>> +			*p = 'h';
> >>> +			break;
> >>> +		case 'P':
> >>> +			*p = 'p';
> >>> +			break;
> >>> +		case 'T':
> >>> +		default:
> >>> +			break;
> >>> +		}
> >>> +	}
> >>> +}
> >>
> >> You parse '%%H' incorrectly.
> >
> > I’m pretty sure I don’t.
> 
> Aggh, I see it now.  The first line should be
> 
>  for (p = strchr(fmt, '%'); ...
> 
> as I would have noticed with even a little testing.

Actually, we are both failing. You _do_ parse %%H right, but you don't parse "WHO" correctly. So yours is a different bug than what I thought. :)

-Peff