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

[Namhyung Kim <namhyung@gmail.com>]

2011-03-10 (목), 01:43 -0800, Junio C Hamano:
> 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.
> 

OK. But I'm still wonder why the value is "8" instead of
"7" (DEFAULT_ABBREV).


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

Right. But I'm not sure what the end result should be if the guard is
set and/or --no-uniq is given. It might be related to the patch 1/2.

Say the unique length is 4 and the guard is 3. What do you expect from
the following command?

	$ git blame --abbrev=5 -- $path
	$ git blame --abbrev=5 --no-uniq -- $path

I think it should be 8 and 5. But possible alternatives are (7, 5),
(5, 5), (8, 8) and (7, 8).

Let me think of another example. In this case the unique length is 8,
the guard is 4 and the commands are same.

I think it should be (9, 5). And corresponding alternatives are (12, 5),
(8, 5), (12, 9) and (12, 9).


My logic can be described like:

	default output 	= max((c + g), u)
	no-uniq output 	= c

where 'l' is the caller-given length, 'g' is the guard length, and 'u'
is the unique length.

Alternatives are:

(a)	default output = max((u + g), c)
	no-uniq output = c

(b)	default output = max(c, u)
	no-uniq output = c

(c)	default output = (c > u) ? (c + g) : (u + g)
	no-uniq output = c + g

(d)	default output = max((u + g), c)
	no-uniq output = c + g


Maybe we can make more combination if you want. What do you think?



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

How about "--fixed" ?

Thanks.


-- 
Regards,
Namhyung Kim