git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Sam Vilain <sam@vilain.net>
Cc: Jason Sewall <jasonsewall@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] git-mergetool: add support for ediff
Date: Mon, 2 Jul 2007 21:09:55 -0400	[thread overview]
Message-ID: <20070703010955.GA5322@thunk.org> (raw)
In-Reply-To: <46898815.6030607@vilain.net>

On Tue, Jul 03, 2007 at 11:19:49AM +1200, Sam Vilain wrote:
> Thanks for that, it mostly works, however it doesn't seem to notice if I
> abort without making the merge complete (on emacs21).  In my smartmerge
> script (http://utsl.gen.nz/scripts/smartmerge) I detect this condition
> based on the presence of merge markers, possibly dubious but pragmatic.

Hmm, well, here's a way of fixing it.  (See attached, below.)  It adds
a new command 'x', which when you hit it in the ediff control window,
exits with a error status of '1', indicating that the merge has
failed.  This is something which emerge, kdiff3, tkdiff, et. al all
support; but which ediff doesn't.

> I still don't really understand why having to save the merged buffer and
> exit is such a huge issue.  Already I have to select "-t emerge" to get
> emerge.  I would have thought it would be better to just make the other
> mode available, and let the user figure it out.

I'm just exploring alternatives.  Basically, it just seems interesting
that ediff has a lot of nice features, but also has some incredibly
user-hostile features.  The first time I tried using ediff, I indeed
tried saving the buffer and exiting it.  That's when I discovered that
after I changed the focus to the merge window and saved it, when I
tried typing ^X^C, the exit failed with the error message "Attempt to
delete a surrogate minibuffer frame".  That's the sort of thing that
will cause non-elisp programmers to run screaming off into the
distance.

So if you are going to save the merge the buffer and exit, you *have*
to use the 'q' command, and endure the loads of stupid questions
issued by ediff, OR, you can discover that ^X^C in the ediff control
window doesn't actually cause emacs to exit, but it does make the
ediff control window go away.  (Which is another insane bit of ediff's
UI design... why should ^X^C do something completely different in the
ediff control window?!?)

So yeah, we can add ediff as an optional support that people have to
explicitly request, but quite frankly, having played with it, I don't
know why anyone would use it without a huge number of fix ups, which
is why I was trying to make ediff actually be usable for someone who
doesn't mind typing ^X^C twice, for no good reason, after figuring out
that this illogical thing is what you actually need to do to exit
ediff.  (I actually read the help text first, so I got treated to the
really annoying ediff-quit behavior before I figured out the double
^X^C trick.)

						- Ted

;; use-ediff-instead.el
;;
;; This emacs lisp snippet should be placed in your .emacs.el file in
;; order to use the ediff package instead of emerge for git-mergetool.
;; Ediff has more whiz-bang features, but unfortunately it doesn't
;; integrate well with shell scripts that try to invoke ediff from an
;; emacs shell invocation.  This script tries to address these problems.

(defun ediff-write-merge-buffer ()
  (let ((file ediff-merge-store-file))
    (set-buffer ediff-buffer-C)
    (write-region (point-min) (point-max) file)
    (message "Merge buffer saved in: %s" file)
    (set-buffer-modified-p nil)
    (sit-for 1)))

(defun ediff-abort ()
  "Abort the ediff session without a non-zero exit status"
  (interactive)
  (kill-emacs 1))

(defun ediff-setup-abort ()
  (define-key ediff-mode-map "x" 'ediff-abort))

(defun emerge-files-command ()
  (let ((file-a (nth 0 command-line-args-left))
	(file-b (nth 1 command-line-args-left))
	(file-out (nth 2 command-line-args-left)))
    (setq command-line-args-left (nthcdr 3 command-line-args-left))
    (setq ediff-quit-hook 'kill-emacs
	  ediff-quit-merge-hook 'ediff-write-merge-buffer
	  ediff-keymap-setup-hook 'ediff-setup-abort)
    (ediff-merge-files file-a file-b  nil file-out)))

(defun emerge-files-with-ancestor-command ()
  (let (file-a file-b file-anc file-out)
    ;; check for a -a flag, for filemerge compatibility
    (if (string= (car command-line-args-left) "-a")
	;; arguments are "-a ancestor file-a file-b file-out"
	(progn
	  (setq file-a (nth 2 command-line-args-left))
	  (setq file-b (nth 3 command-line-args-left))
	  (setq file-anc (nth 1 command-line-args-left))
	  (setq file-out (nth 4 command-line-args-left))
	  (setq command-line-args-left (nthcdr 5 command-line-args-left)))
        ;; arguments are "file-a file-b ancestor file-out"
        (setq file-a (nth 0 command-line-args-left))
        (setq file-b (nth 1 command-line-args-left))
        (setq file-anc (nth 2 command-line-args-left))
        (setq file-out (nth 3 command-line-args-left))
        (setq command-line-args-left (nthcdr 4 command-line-args-left)))
    (setq ediff-quit-hook 'kill-emacs
	  ediff-quit-merge-hook 'ediff-write-merge-buffer
	  ediff-keymap-setup-hook 'ediff-setup-abort)
    (ediff-merge-files-with-ancestor file-a file-b file-anc nil file-out)))

;; End of use-ediff-instead.el

  reply	other threads:[~2007-07-03  1:10 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
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 [this message]
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=20070703010955.GA5322@thunk.org \
    --to=tytso@mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jasonsewall@gmail.com \
    --cc=sam@vilain.net \
    /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).