Re: [PATCH 2/2] blame: honor core.abbrev

[Namhyung Kim <namhyung@gmail.com>]

Hello, Junio-san.

2011-04-01 (금), 15:44 -0700, Junio C Hamano:
> Namhyung Kim <namhyung@gmail.com> writes:
> 
> > If user sets config.abbrev option, use it as if --abbrev was given.
> > We can't set abbrev to default_abbrev unconditionally IMHO, because
> > historically default abbrev length of the blame command is 8 and
> > DEFAULT_ABBREV is 7.
> 
> Isn't the one-letter difference because we sometimes need to show the
> boundary commit with a caret at the beginning?
> 

Yeah, I guessed so.


> I think the way this patch initializes orig_abbrev using DEFAULT_ABBREV is
> wrong (at that point, I don't think you have called git_config() to get
> the user config for DEFAULT_ABBREV).
> 

My intention was that I want to check whether the user sets the option
or not and apply the value only if it is set (ig. different from "7").
Please see below.


> See the patch to describe.c in dce9648 (Make the default abbrev length
> configurable, 2010-10-28) for the right way to do this.
> 
>  - initialize the variable to -1;
>  - call git_config() to get correct value in DEFAULT_ABBREV;
>  - call parse_options() to potentially update the variable; then
>  - if variable is still -1, assign DEFAULT_ABBREV to it.
> 
> After all that, add 1 to it to account for the possible boundary caret.
> 

I saw the patch already, and, yes, it would be a simple and probably the
right way to do. But it could give a bit of confusion to the users IMHO.
Say, A user specify --abbbrev=10 then he/she might expect 10 hexdigits
in the output but actually [s]he will get 11.

So I decided to use the number as is, not adding 1. And this would be a
constant behavior from the patch 1/2 and all other git command which use
--abbrev option, I guess. But I kept to use 8 in case the user doesn't
give the config or command-line option, just for backward compatibility.

I'm not sure this is really a concern. Maybe we can give them 11 as you
said and document the behavior in the manual page.

Thanks.


-- 
Regards,
Namhyung Kim