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

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

 



Namhyung Kim <namhyung@xxxxxxxxx> 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.

--
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]