[PATCH] Warn the users when more than 3 '-C' given.

[PATCH] Warn the users when more than 3 '-C' given.

[Bo Yang]

Output a warning message to users when there are more than
3 '-C' options given. And ignore the numeric argument value
provided by the additional '-C' options.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
---
 builtin/blame.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index fc15863..e8ed547 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2165,6 +2165,15 @@ static int blame_copy_callback(const struct option *option, const char *arg, int
 	int *opt = option->value;
 
 	/*
+	 * Warn the users when more than 3 '-C' options are given and
+	 * ignore the corresponding numeric argument of it.
+	 */
+	if (*opt & PICKAXE_BLAME_COPY_HARDEST) {
+		warning("The additional '-C' above 3 is not supported.");
+		return 0;
+	}
+
+	/*
 	 * -C enables copy from removed files;
 	 * -C -C enables copy from existing files, but only
 	 *       when blaming a new file;
-- 
1.7.0.2.273.gc2413.dirty

Re: [PATCH] Warn the users when more than 3 '-C' given.

[Junio C Hamano]

Bo Yang <struggleyb.nku@gmail.com> writes:

> Output a warning message to users when there are more than
> 3 '-C' options given. And ignore the numeric argument value
> provided by the additional '-C' options.

How were you bitten by the lack of this warning?  You gave four or five to
see how output would change, spent sleepless nights but couldn't figure
out what the differences between third and fourth levels are, and wasted
too much time?

IOW, what does this fix?

I personally do not see much value in this patch.  It would be just a
hindrance to remember to remove this hunk when somebody improves the
algorithm to add fourth level of detail to the inspection logic.

Re: [PATCH] Warn the users when more than 3 '-C' given.

[Yann Dirson]

Le Sat, 10 Apr 2010 12:12:58 -0700,
Junio C Hamano <gitster@pobox.com> a écrit :

> Bo Yang <struggleyb.nku@gmail.com> writes:
> 
> > Output a warning message to users when there are more than
> > 3 '-C' options given. And ignore the numeric argument value
> > provided by the additional '-C' options.
> 
> How were you bitten by the lack of this warning?  You gave four or
> five to see how output would change, spent sleepless nights but
> couldn't figure out what the differences between third and fourth
> levels are, and wasted too much time?

That sounding a bit harsh, I guess it is my turn to take the blame for
suggesting this in last week's thread :)


> IOW, what does this fix?

One practical advantage of this warning would be, in the very case of
adding meaning to an additional -C, that a user trying to use it on an
older version of git would get a warning that the program might not
indeed to what the user requested.

However, my first feeling was simply that, while it is usually harmless
to let the user specify a flag several time, when it changes nothing,
the situation is different when repetition of the flag is important -
it is closer to an invalid flag combination.

In fact, I even dislike that use of repetitive -C.  One could argue
that it is much like repetition of -v used in various programs to raise
verbosity.  But well, in our case, it is much more than just increasing
the level of details, it makes it use a different mechanism - even if
each time it is a superset of the previous one.

And what if someone comes with an idea of a "level of -C" that indeed
lays between two existing ones ?  Will we shift the meaning of the
existing ones ?  And what about one "level" that would not strictly fit
in the existing "superset" chain ?

What about instead using a more descriptive flag ?  That would be more
verbose typing, but then we can still keep the existing flags for
backward compatibility, and we also have shell command-line completion.

I'd think about something like:
-C -C     -> -Cunmodified (that one also for diff)
-C -C -C  -> -Chistory

I could also argue that "blame -M" could also be better placed as a -C
variant (it is also supposed to detect some copies), and could have as
fullname something like "blame -Csamefile".


> I personally do not see much value in this patch.  It would be just a
> hindrance to remember to remove this hunk when somebody improves the
> algorithm to add fourth level of detail to the inspection logic.

Well, the warning should trigger the 1st time that somebody tests his
fourth -C, right ?

-- 
Yann