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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jay Soffian <jaysoffian@xxxxxxxxx> 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?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]