Re: [PATCH v2] blame: honor core.abbrevguard

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

 



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.

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.

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.

So thanks for a patch, but I don't think we would want to take it in the
current shape.
--
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]