All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Tanguy Ortolo <tanguy+debian@ortolo.eu>,
	git@vger.kernel.org, Sebastian Schuberth <sschuberth@gmail.com>,
	Charles Bailey <charles@hashpling.org>
Subject: Re: git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED
Date: Thu, 11 Aug 2011 01:38:38 -0700	[thread overview]
Message-ID: <20110811083835.GA29507@gmail.com> (raw)
In-Reply-To: <20110810161211.GC4076@elie.gateway.2wire.net>

On Wed, Aug 10, 2011 at 11:12:11AM -0500, Jonathan Nieder wrote:
> (+cc: some relevant people)
> Hi,
> 
> Tanguy Ortolo wrote[1]:
> 
> > git-mergetool ideally uses tools that work with 4 files: BASE, LOCAL,
> > REMOTE, which are the usual original and two new version of the file,
> > and MERGED, which is where the tool is supposed to write the result of
> > the merge.
> >
> > The problem is that most tools, at least graphical ones, specifically
> > meld, can only work with three files, as they save the result to the
> > original file.
> >
> > git-mergetool currently handles this situation by passing MERGED LOCAL
> > REMOTE to the tool. This could be fine, but unfortunately MERGE contains
> > the conflicts, formatted for manual resolution, so it is not really
> > appropriate as an original file.
> >
> > I think it would be better to wrap such merge tools by:
> > 1. passing them BASE LOCAL REMOTE;
> > 2. checking whether or not BASE hase been modified:
> >    * if it has, then copying it to MERGED,
> >    * if it has not, exiting with return code 1 (merge failed).
> > This check can be by either saving and comparing the mdate, or perhaps
> > the SHA-1 hash of the BASE file.
> >
> > If this sounds good enough, I can dive into git-mergetoo--lib and
> > implement it. In the meantime, here is an example of a custom merge tool
> > that wraps meld for that purpose.
> 
> I think you forgot to include the example.  Anyway, at first glance it
> sounds like a sensible idea.  David et al: thoughts?

I think it sounds like a good thing for certain tools.
Sebastian mentioned it being fine in ecmerge and bc3.
xxdiff also lets you specify the output file, so it
probably wouldn't need it either, I think.

If the patch touches individual tools that need it, e.g. meld,
I say go for it.

> Regards,
> Jonathan
> 
> [1] http://bugs.debian.org/637355

cheers,
-- 
					David

  parent reply	other threads:[~2011-08-11  8:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20110810160356.GA32126@ortolo.eu>
2011-08-10 16:12 ` git-mergetool: wrap tools with 3 files only to use the BASE file instead of MERGED Jonathan Nieder
2011-08-10 17:24   ` Tanguy Ortolo
2011-08-10 19:23   ` Sebastian Schuberth
2011-08-11  8:38   ` David Aguilar [this message]
2011-08-11  8:49     ` Jonathan Nieder
2011-08-11  9:37       ` David Aguilar
2011-08-11  9:48         ` Sebastian Schuberth

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=20110811083835.GA29507@gmail.com \
    --to=davvid@gmail.com \
    --cc=charles@hashpling.org \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=sschuberth@gmail.com \
    --cc=tanguy+debian@ortolo.eu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.