Re: [PATCH/RFC][GSoC] make "git diff --no-index $directory $file" DWIM better.

> Yurii Shevtsov <ungetch@gmail.com> writes:
>
>>> ...  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.
>>
>> So you want me to convert args ("diff D/ F" into "diff D/F F") before
>> calling queue_diff()? But why?
>
> Because it is wrong to do this inside queue_diff()?
>
> Have you actually read what I wrote, tried the above sample
> scenario, and thought what is happening in the codepath?
>
> When the user asks to compare directory a/ and b/, the top-level
> diff_no_index() would have paths[0]=="a" and paths[1]=="b", and
> queue_diff() is called with these in name1 and name2.  Once it
> learns that both of these are directories, it _recurses_ into itself
> by appending the paths in these directories after these two names.
> It finds that both of these directories have "sub" underneath, so it
> makes a recursive call to itself to compare "a/sub" and "b/sub".
>
> That call would notice that one is a directory and the other is
> not.  That is where you are getting the "f/d conflict" error.
>
> At that point, do you think it is sensible to rewrite that recursed
> part of the diff into a comparison between "a/sub/sub" (which does
> not exist, and which the user did not mean to compare with b/sub)
> and "b/sub" (which is a file)?  I hope not.
>
>> queue_diff() already check args' types and decides which
>> comparison to do.
>
> Yes, and I already hinted that that is an independent issue we may
> want to fix, which I suspect is larger than GSoC Micro.  Also the
> fix would be different.  Right now, it checks the types of paths and
> then refuses to compare a directory and a file.  If we wanted to
> change it to closer to what the rest of Git does, we would want it
> to report that the directory and everything under it are removed and
> then a new file is created (if the directory is on the left hand
> side of the comparision and the file is on the right hand side) or
> the other way around.  That will not involve "append the name of the
> file at the end of the directory".
>
> In short, "append the name of the file at the end of the directory"
> logic has no place to live inside queue_diff() which handles the
> recursion part of the operation.