git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tim Henigan <tim.henigan@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: David Aguilar <davvid@gmail.com>,
	git@vger.kernel.org, ramsay@ramsay1.demon.co.uk
Subject: Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs
Date: Wed, 18 Apr 2012 14:28:18 -0400	[thread overview]
Message-ID: <CAFouetgWpyUC9SPo_QwpESrbfib7ct111WesKPP14HQ+SqpFaQ@mail.gmail.com> (raw)
In-Reply-To: <7vsjg1knwr.fsf@alter.siamese.dyndns.org>

On Wed, Apr 18, 2012 at 12:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> 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.

  reply	other threads:[~2012-04-18 18:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-13 16:36 [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs Tim Henigan
2012-04-15 22:20 ` David Aguilar
2012-04-16  1:01   ` David Aguilar
2012-04-16  8:16     ` David Aguilar
2012-04-17 13:25     ` Tim Henigan
2012-04-18  3:23       ` David Aguilar
2012-04-18 13:13         ` Tim Henigan
2012-04-18 16:25           ` Junio C Hamano
2012-04-18 18:28             ` Tim Henigan [this message]
2012-04-18 19:38               ` Junio C Hamano
2012-04-19 17:11                 ` Tim Henigan
2012-04-19 18:49                   ` Junio C Hamano
2012-04-20  7:34                     ` David Aguilar
2012-04-20 16:58                       ` Tim Henigan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFouetgWpyUC9SPo_QwpESrbfib7ct111WesKPP14HQ+SqpFaQ@mail.gmail.com \
    --to=tim.henigan@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).