Re: [PATCH 2/2] blame: introduce -u/--unique option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 10, 2011, 09:43 UTC
Message-ID: <7vhbbb5mzo.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1299737621.1496.55.camel@leonhard>

Namhyung Kim <namhyung@gmail.com> writes:

> Hmm... I thought about that too. But you mean that you only want
> --abbrev instead of -u, right?

Yeah, if it can be shown that ensuring the uniqueness in addition to uniform length with negligible cost, I don't think there is any reason to have this as an option. So perhaps the minimum change would be to keep the current "8" as a hardwired constant to feed find-unique-abbrev with to find the longest unique prefix as your patch did and use that length when showing the output. Making "8" configurable would become a separate topic.

> My original intention was if user specified --abbrev explicitly, it
> should be honored regardless of the uniqueness. The guard will not be
> used in this situation because the user gave the exact length [s]he
> wants to see.

Hmm, like giving a ridiculously short --abbrev and forcing the output to be ambiguous yet hopefully they can be differenciated by other clues like author and date?

The reason I originally suggested to make the uniqueness an optional feature was primarily output performance, but that is linear with the number of lines in the file and is dwarfed by the history traversal cost, so it is a non-issue, so I then suggested that it doesn't need to be an option. But I think you may have a point--an option to decline uniqueness would also make sense, especially when you want a very short prefix that you know won't be unique and when you don't care about uniqueness.

As all the normal commands that use --abbrev would call f-u-a to ensure uniqueness, the current "cut at fixed length" behaviour is an oddball. It should become an optional behaviour.

The current behaviour, when we eventually have both options, would be:

	$ git blame --abbrev=8 --no-uniq $rev -- $path

and should be the default when neither --abbrev=$n nor --no-uniq is given for backward compatibility. Porcelains are _not_ supposed to be reading from blame output without giving it --porcelain option, but we know we cannot expect sanity from people who script around git.

When you want to give a very short width that wouldn't guarantee uniqueness, and you want to force that width, you would say:

	$ git blame --abbrev=4 --no-uniq $rev -- $path

Without --no-uniq,

	$ git blame --abbrev=4 $rev -- $path

the command may use more hexdigits than 4 to ensure uniqueness, but all commit names are shown with the same width to ensure alignment as well.

I kind of like that. While I don't think "--no-uniq" is the best name for that option, I think --abbrev everywhere else implies uniqueness so it is not a good idea to require "-u" option for unique output and give ambiguous output without one. It would hurt consistency in the UI.