Re: [PATCHv3] replace: parse revision argument for -d

[Michael J Gruber <git@drmicha.warpmail.net>]

Jeff King venit, vidit, dixit 12.11.2012 21:42:
> On Mon, Nov 12, 2012 at 03:18:02PM +0100, Michael J Gruber wrote:
> 
>> 'git replace' parses the revision arguments when it creates replacements
>> (so that a sha1 can be abbreviated, e.g.) but not when deleting
>> replacements.
>>
>> Make it parse the argument to 'replace -d' in the same way.
>>
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>>
>> Notes:
>>     v3 safeguards the hex buffer against reuse
> 
> Thanks, I don't see any other functional problems.
> 
>> diff --git a/builtin/replace.c b/builtin/replace.c
>> index e3aaf70..33e6ec3 100644
>> --- a/builtin/replace.c
>> +++ b/builtin/replace.c
>> @@ -46,24 +46,28 @@ typedef int (*each_replace_name_fn)(const char *name, const char *ref,
>>  
>>  static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
>>  {
>> -	const char **p;
>> +	const char **p, *q;
> 
> I find this readable today, but I wonder if in six months we will wonder
> what in the world "q" means. Maybe "short_refname" or something would be
> appropriate?

That would be sooo inappropriate! ;)

Maybe "full_hex"?

I should also do away with the first replacement which really made sense
in v1 only.

v4 to follow.

Michael