All of lore.kernel.org
 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 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.