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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello, Junio-san.

2011-04-01 (ê), 15:44 -0700, Junio C Hamano:
> Namhyung Kim <namhyung@xxxxxxxxx> 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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]