Mike Hommey <mh@xxxxxxxxxxxx> writes:
> While -C implies -M, it is quite common to see both on example command lines
> here and there. The unintuitive thing is that if -M appears after -C, then
> copy detection is turned off because of how the command line arguments are
> handled.
This is deliberate, see below.
> Change this so that when both -C and -M appear, whatever their order, copy
> detection is on.
>
> Signed-off-by: Mike Hommey <mh@xxxxxxxxxxxx>
> ---
>
> Interestingly, I even found mentions of -C -M in this order for benchmarks,
> on this very list (see 6555655.XSJ9EnW4BY@mako).
Aside from the general warning against taking everything you see on
this list as true, I think you are looking at an orange and talking
about an apple. $gmane/244581 is about "git blame", and "-M/-C"
over there mean completely different things.
Imagine you have a file a/b/c/d/e/f/g/h/, where each alphabet
represents a line with more meaningful contents than a single-liner,
and slash represents an end of line, and you have changed the
contents of the file to e/f/g/h/a/b/c/d/.
"blame -M" is to tell the command to notice that you moved the first
four lines to the end (i.e. you did not do anything original and
just moved lines). Because this needs more processing to notice,
the feature is triggered by a "-M" option (you would see something
like "e/f/g/h/ came from the original and then a/b/c/d/ are newly
added" without the option). "-M" in "blame" is about showing such
movement of lines within the same file [*1*].
On the other hand "-C" in "blame" is about noticing contents that
were copied from other files.
In the context of "git blame", "-C" and "-M" control orthogonal
concepts and it makes sense to use only one but not the other, or
both.
But "-M" given to "git diff" is not about such an orthogonal
concept.
Even though its source candidates are narrower than that of "-C" in
that "-M" chooses only from the set of files that disappeared while
"-C" also chooses from files that remain after the change, they are
both about "which file did the whole thing come from?", and it makes
sense to have progression of "-M" < "-C" < "-C -C". You can think
of these as a short-hand for --rename-copy-level={0,1,2,3} option
(where the level 0 -- do not do anything -- corresponds to giving no
"-M/-C"). It can be argued both ways: either we take the maximum of
the values given to --rename-copy-level options (which corresponds
to what your patch attempts to do), or the last one wins. We happen
to have implemented the latter long time ago and that is how
existing users expect things to work.
So, I dunno. I am saying that the semantics the current code gives
is *not* the only possibly correct one, and I am not saying that
your alternative interpretation is *wrong*, but I am not sure if it
makes sense to switch the semantics this late in the game.
[Footnote]
*1* "diff" cannot do anything magical about such a change. It can
only say that you removed the first four lines from the original,
and then added new four lines at the end (or it may say you added
new four lines at the beginning and removed four lines at the end).
There is no way for "diff" to say that these new four lines have any
relation to what you removed from the beginning of the file, with
any combination of options. This is inherent in its output format,
which is limited to express only deletions and additions.
> diff.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index d1bd534..9081fd8 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3670,7 +3670,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
> !strcmp(arg, "--find-renames")) {
> if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
> return error("invalid argument to -M: %s", arg+2);
> - options->detect_rename = DIFF_DETECT_RENAME;
> + if (options->detect_rename != DIFF_DETECT_COPY)
> + options->detect_rename = DIFF_DETECT_RENAME;
> }
> else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
> options->irreversible_delete = 1;
--
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