On Wed, Apr 18, 2012 at 12:25 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Tim Henigan <tim.henigan@xxxxxxxxx> writes: > >> So now we must decide how to handle deal with this use case. It seems >> there are two options: >> >> 1) Append '--no-renames' to the end of the 'git diff --raw' argument >> list. This will override any '-C' or '-M' settings. This is a simple >> solution, but it loses some information about copies and renames. > > Or not use Porcelain "git diff", but use the plumbing "git diff-index" or > "git diff-files" so that you won't get bitten by such end user settings. Looking back on it now, I agree that it would have been better to use the plumbing commands from the beginning. Changing from the porcelain to the plumbing commands will require new logic to parse the diff options to figure out which of 'diff-index', 'diff-files' or 'diff-tree' should be called. We may also want to add support for some specific standard diff options (like '-R'). For now, would you object to an updated patch that simply detects and ignores options that change the output of 'git diff --raw'? Or do you think that we need to switch to the plumbing commands before the directory diff feature can be called stable? I was planning to look for the following: --find-renames (and -M) --find-copies (and -C) --cc (and -c) If any of the above are detected, 'difftool' would print a warning that the option is not supported and then prune it from the arguments passed to 'git diff --raw'. > In either case, this "feature", by feeding two entire trees to an external > program, makes it the responsibility of that external program to match up > files in these two trees, so we shouldn't be doing rename detection > ourselves at all. I agree that we should not try to do it in the 'difftool' command. Unfortunately, it appears that none of the external tools can detect renames or copies. -- 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