Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
>> --- a/diff-no-index.c
>> +++ b/diff-no-index.c
>> @@ -97,8 +97,25 @@ static int queue_diff(struct diff_options *o,
>> if (get_mode(name1, &mode1) || get_mode(name2, &mode2))
>> return -1;
>>
>> - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))
>> - return error("file/directory conflict: %s, %s", name1, name2);
>
> I'm surprised to see this error message totally go away. The idea of the
> microproject was to DWIM (do what I mean) better, but the dwim should
> apply only when $directory/$file actually exists. Otherwise, the error
> message should actually be raised.
I actually think this check, when we really fixed "diff --no-index"
to work sensibly, should go away and be replaced with something
sensible. As it stands now, even before we think about dwimming
"diff D/ F" into "diff D/F F", a simple formulation like this will
error out.
$ mkdir -p a/sub b
$ touch a/file b/file b/sub a/sub/file
$ git diff --no-index a b
error: file/directory conflict: a/sub, b/sub
Admittedly, that is how ordinary "diff -r" works, but I am not sure
if we want to emulate that aspect of GNU diff. If the old tree a
has a directory 'sub' with 'file' under it (i.e. a/sub/file) where
the new tree has a file at 'sub', then the recursive diff can show
the removal of sub/file and creation of sub, no? That is what we
show for normal "git diff".
But I _think_ fixing that is way outside the scope of GSoC Micro.
And patching this function, because it is recursively called from
within it, to "dwim" is simply wrong. When we see a/sub that is a
directory and b/sub that is a file, we do *NOT* want to rewrite the
comparison to comparison between a/sub/sub and b/sub.
What needs to be fixed for the Micro is the top-level call to
queue_diff() that is made blindly between paths[0] and paths[1]
without checking what kind of things these are.
--
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