Re: [PATCH] git-mergetool: add support for ediff

[Theodore Tso <tytso@mit.edu>]

On Fri, Jun 29, 2007 at 12:03:28AM -0400, Theodore Tso wrote:
> I'll have to look at the two and see why people like one over the
> other, and then we'll have to pick which one should be the default.
> Although as I've said, past a certain point people should just put
> their personal preference in .gitconfig.

After looking at ediff, it is definitely the more polished and
featureful compared to emerge --- except in one critical area, which
is calling as a mergeing tool from a shell script or command line.
Ediff fundamentally assumes that it fired off from inside an emacs
environment, whereas emerge is much friendly as an external merge
program. 

This can be shown in the relatively easy way emerge can be run from
the command-line:

	emacs -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$path"

... where as with ediff, you have to run it this way:

	emacs --eval "(ediff-merge-files-with-ancestor \"$LOCAL\" \"$REMOTE\" \"$BASE\" nil \"$path\")"

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:

Quit this Ediff session? (y or n)
File /usr/projects/git/test/testfile.c exists, overwrite? (y or n)
Merge buffer saved in /usr/projects/git/test/testfile.c
<delay for 3 annoying seconds>
Merge buffer saved.  Now kill the buffer? (y or n)

... and then it leaves you in the emacs window, and you have to type
^X^C by hand.

So while ediff is more featureful, its integration is so lacking that
it is incredibly annoying to use.

Which leaves us with the interesting question.  We could just
integrate it, but not make it the default (the above makes ediff just
far too annoying for a user who is not expecting it).  

Alternatively, we could patch around the problem.  The following emacs
lisp code fixes the ediff issues:

(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)))

(setq ediff-quit-hook 'kill-emacs
      ediff-quit-merge-hook 'ediff-write-merge-buffer)

But the only clean way of adding that to git-mergetool would be something like this:

	emacs --eval "(progn (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))) (setq ediff-quit-hook 'kill-emacs ediff-quit-merge-hook 'ediff-write-merge-buffer) (ediff-merge-files-with-ancestor \"$LOCAL\" \"$REMOTE\" \"$BASE\" nil \"$path\")"

But that seems too ugly to live, and it could break in the future if
ediff ever changes some of its internal variables.


Alternatively, we could file a bug report with the ediff folks, and
request that they add an 'ediff-files-with-ancestor-command and
'ediff-files-command just as emerge does.  The problem with that
approach is that ediff is shipped with emacs, and emacs has a release
cycle measured in **years**.


So my current thinking is that ediff will *not* be the default for
git-mergetool if emacs is present, and that emerge will be used for
now, because of these problems.

Comments?

						- Ted