Hi,
2011-03-07 (ì), 16:59 -0800, Junio C Hamano:
> Namhyung Kim <namhyung@xxxxxxxxx> writes:
>
> > Commit 72a5b561 ("core.abbrevguard: Ensure short object names stay
> > unique a bit longer") introduced this config variable to make object
> > name unambiguously. Use it.
> >
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxx>
> > ---
> > v2:
> > - re-init 'length' in every loop
>
> In this codepath, we only aim to make sure that the shortened object name
> is of the same length. By choosing hardcoded 8, the original clearly
> declares that we do not _care_ about the uniqueness.
>
Just out of curiousity, where did the number come from? I guess
DEFAULT_ABBREV + '^' ?
> The abbrev-guard that adds to the length that ensures the uniqueness in
> the particular repository has a very different semantics. It only makes
> sense if you add to a length that you know would make the object names
> minimally unique. Adding it to hardcoded 8 does not make it less
> ambiguous the same way as the configuration variable intends to do.
>
> So I am not entirely happy with this patch.
>
> Besides, when OUTPUT_LONG_OBJECT_NAME is specified, the value this
> variable holds is _not_ "uniq_length", so the name of the variable is not
> quite correct. We colloquially call the object name "sha1" in the code,
> so "sha1_length" would be a better name for it.
>
OK. I was not happy with the name, too. :)
> If we _were_ to do a change that uses the configuration variable, I would
> imagine that it would be a part of a change that makes sure that the
> shortened object names in the output actually uniquely identify the
> commits. It would involve find_unique_abbrev() for as many commits as the
> number of lines in the blamed range at most (and at that point the abbrev
> guard would automatically taken into account because find_unique_abbrev()
> already does so) before calling "output()"; I suspect that find_alignment()
> is the right function to do so, as that is where the column alignment
> logic all happens.
>
> It would probably need to be introduced as a new feature, with its own
> command line option, similar to -l option that forces the full 40-hex
> output.
>
Looks Great. How about -u/--unique? I'm also thinking about --abbrev
too, isn't it useful at all?
> So thanks for a patch, but I don't think we would want to take it in the
> current shape.
Thanks for your kindly comment.
--
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