Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option

[David Aguilar <davvid@gmail.com>]

(continuing an old thread...)
http://thread.gmane.org/gmane.comp.version-control.git/134906/focus=135006

On Wed, Dec 09, 2009 at 04:08:55PM -0800, Junio C Hamano wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
> 
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> > index 5b62785..5b29fef 100644
> > --- a/git-mergetool--lib.sh
> > +++ b/git-mergetool--lib.sh
> > @@ -46,7 +46,8 @@ check_unchanged () {
> >  valid_tool () {
> >  	case "$1" in
> >  	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
> > -	emerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge)
> > +	emerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge | \
> > +	diffmerge)
> >  		;; # happy
> >  	tortoisemerge)
> >  		if ! merge_mode; then
> 
> As we are in pre-release feature freeze, it doesn't matter very much if I
> take this patch now, so I'll let it sit in the list archive for now.
> 
> But I have to wonder about the maintainability of this file, if we have to
> add every time somebody finds yet another diff/merge backends that could
> be used, even a closed-source one.
> 
> There are only a handful of entry points that mergetool--lib defines, and
> by overriding what should happen when these entry points are called, an end
> user should be able to tell mergetool/difftool to use a new backend.
> 
> Perhaps it is a better approach to first eject bulk of code for the
> backends we currently support under these case statements into separate
> files, one per backend, move them to mergetool/ subdirectory in the source
> tree, install them as "$(share)/git-core/mergetool/$toolname", and at
> runtime source them?  That way, a patch to add a new backend can be as
> simple as adding a new file in mergetool/ and doing nothing else.  Also an
> end user can privately add support to a new backend much more easily.
> 
> Anybody want to try that approach?

I've started working on splitting this file apart and it is
coming along nicely.  I should have some patches soon.

We're approaching feature freeze so I will hold onto them for a
bit.  I worked the "meld should use --output with >= 1.5.0"
check into it too, which worked out nicely with the refactored
setup as that logic is neatly tucked away into the meld file.

Is it okay if we install the files into
$(git --exec-path)/mergetools/ instead?  I didn't spot an
obvious way to construct $(share)/git-core/ at runtime
(without dirname tricks that assume share=$(prefix)/share).
-- 
					David