git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jason Sewall <jasonsewall@gmail.com>,
	Sam Vilain <sam.vilain@catalyst.net.nz>,
	git@vger.kernel.org
Subject: Re: [PATCH] git-mergetool: add support for ediff
Date: Sun, 1 Jul 2007 23:05:21 -0400	[thread overview]
Message-ID: <20070702030521.GA4798@thunk.org> (raw)
In-Reply-To: <7v1wfr1qn8.fsf@assigned-by-dhcp.cox.net>

On Sun, Jul 01, 2007 at 07:32:59PM -0700, Junio C Hamano wrote:
> Theodore Tso <tytso@mit.edu> writes:
> 
> > Unfortunately, it's not enough.  Ediff doesn't have an "abort" command
> > which returns a non-zero exit status, and when you use the "quit"
> > command, it asks you a series of obnoxious questions:
> >
> > ...
> > Alternatively, we could patch around the problem.  The following emacs
> > lisp code fixes the ediff issues:
> 
> But that would be changing the behaviour globally, and not
> limited to the particular session invoked from git-mergetool,
> wouldn't it?  If that is the case it would be a hard sell to
> Emacs users, especially the ones that keep their Emacs running
> forever and have emacsclient as their EDITOR, I would think.

The emacs lisp code I gave there was the minimal necessary so it
could be passed on the command-line; I was trying to keep it small.

Obviously, the patch that would have to get sent to the ediff folks
would have to be much more generalized --- in fact, probably the right
thing to do is to send a full patch that actually implemented
ediff-merge-files-command and ediff-merge-files-with-ancestoers-commands.

As far as people using emacsclient as their editor, it would be simple
enough to have the emacs lisp code test to see if
server-buffer-clients is non-nill; if it is, then we know that this
merge request was trigered by emacsclient, and so (server-done) should
be called instead of (kill-emacs).  Emerge does not do this; arguably
this is a bug in emerge.

The other way we could deal with this problem is to fire up a separate
emacs even if EDITOR is emacsclient, on the theory that
EDITOR=emacsclient meants that the user prefers emacs, but it doesn't
necessarily mean that we have to *use* emacsclient, especially when
emerge currently doesn't DTRT with emacsclient.

One thing that did cross my mind is that we could put code which
patched ediff.el and emerge.el in /usr/share/git/lisp/... and then
passed called emacs with something like this "emacs -l
$sharedir/lisp/ediff-patches.el ...".  But this implies packaging
emacs lisp files with git, and I'm not at ALL sure we want to go
there.  Personally, I still like kdiff3 as my personal favorite
mergetool, and given that emacs starts up pretty fast these days, I've
given up on emacsclient, but I know there are certainly people who use
them.

(Mmmm...., I just pulled down an early emacs 23 snapshot with Xft
support enabled, so I can enjoy the anti-aliased font goodness.  Even
with all of the Gtk and Xft bloat, the emacs 23 snapshot is still
quick snappy to fire up.)

					- Ted

  reply	other threads:[~2007-07-02  3:05 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-29  1:00 [PATCH] git-mergetool: add support for ediff Sam Vilain
2007-06-29  1:31 ` Jason Sewall
2007-06-29  4:03   ` Theodore Tso
2007-07-02  2:04     ` Theodore Tso
2007-07-02  2:32       ` Junio C Hamano
2007-07-02  3:05         ` Theodore Tso [this message]
2007-07-02  4:49           ` Junio C Hamano
2007-07-02 14:48             ` Theodore Tso
2007-07-02 23:11               ` Junio C Hamano
2007-07-02 21:32       ` Sam Vilain
2007-07-02 21:58         ` Theodore Tso
2007-07-02 22:16           ` Theodore Tso
2007-07-02 23:19             ` Sam Vilain
2007-07-03  1:09               ` Theodore Tso
2007-07-03  6:27                 ` Sam Vilain
2007-07-28  9:22                 ` David Kastrup
2007-07-29  2:38                   ` Theodore Tso
2007-07-29  8:54                     ` David Kastrup
  -- strict thread matches above, loose matches on Subject: below --
2007-06-30  8:56 a bunch of outstanding updates Sam Vilain
2007-06-30  8:56 ` [PATCH] repack: improve documentation on -a option Sam Vilain
2007-06-30  8:56   ` [PATCH] git-svn: use git-log rather than rev-list | xargs cat-file Sam Vilain
2007-06-30  8:56     ` [PATCH] git-svn: cache max revision in rev_db databases Sam Vilain
2007-06-30  8:56       ` [PATCH] GIT-VERSION-GEN: don't convert - delimiter to .'s Sam Vilain
2007-06-30  8:56         ` [PATCH] git-remote: document -n Sam Vilain
2007-06-30  8:56           ` [PATCH] git-remote: allow 'git-remote fetch' as a synonym for 'git fetch' Sam Vilain
2007-06-30  8:56             ` [PATCH] git-merge-ff: fast-forward only merge Sam Vilain
2007-06-30  8:56               ` [PATCH] git-mergetool: add support for ediff Sam Vilain
2007-06-30 17:19                 ` Junio C Hamano
2007-07-01 22:33                   ` Sam Vilain

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=20070702030521.GA4798@thunk.org \
    --to=tytso@mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jasonsewall@gmail.com \
    --cc=sam.vilain@catalyst.net.nz \
    /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).