{"thread":{"id":"21909","subject":"[PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","startedAt":"2009-12-08T20:01:17Z","lastAt":"2011-08-16T17:43:16Z","messageCount":7,"participants":["Jay Soffian","Charles Bailey","Junio C Hamano","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"129570","messageId":"1260302477-49412-1-git-send-email-jaysoffian@gmail.com","threadId":"21909","inReplyTo":null,"subject":"[PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-12-08T20:01:17Z","receivedAt":"2009-12-08T20:01:17Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Add SourceGear DiffMerge to the set of built-in diff/merge tools, and update\nbash completion and documentation.\n---\n Documentation/git-difftool.txt         |    2 +-\n Documentation/git-mergetool.txt        |    2 +-\n Documentation/merge-config.txt         |    4 ++--\n contrib/completion/git-completion.bash |    2 +-\n git-mergetool--lib.sh                  |   22 ++++++++++++++++++++--\n 5 files changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex 8e9aed6..28178da 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -31,7 +31,7 @@ OPTIONS\n \tUse the diff tool specified by <tool>.\n \tValid merge tools are:\n \tkdiff3, kompare, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff,\n-\tecmerge, diffuse, opendiff, p4merge and araxis.\n+\tecmerge, diffuse, opendiff, p4merge, araxis and diffmerge.\n +\n If a diff tool is not specified, 'git-difftool'\n will use the configuration variable `diff.tool`.  If the\ndiff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\nindex 4a6f7f3..7f00269 100644\n--- a/Documentation/git-mergetool.txt\n+++ b/Documentation/git-mergetool.txt\n@@ -27,7 +27,7 @@ OPTIONS\n \tUse the merge resolution program specified by <tool>.\n \tValid merge tools are:\n \tkdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge,\n-\tdiffuse, tortoisemerge, opendiff, p4merge and araxis.\n+\tdiffuse, tortoisemerge, opendiff, p4merge, araxis and diffmerge.\n +\n If a merge resolution program is not specified, 'git-mergetool'\n will use the configuration variable `merge.tool`.  If the\ndiff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt\nindex a403155..a68a205 100644\n--- a/Documentation/merge-config.txt\n+++ b/Documentation/merge-config.txt\n@@ -23,8 +23,8 @@ merge.tool::\n \tControls which merge resolution program is used by\n \tlinkgit:git-mergetool[1].  Valid built-in values are: \"kdiff3\",\n \t\"tkdiff\", \"meld\", \"xxdiff\", \"emerge\", \"vimdiff\", \"gvimdiff\",\n-\t\"diffuse\", \"ecmerge\", \"tortoisemerge\", \"p4merge\", \"araxis\" and\n-\t\"opendiff\".  Any other value is treated is custom merge tool\n+\t\"diffuse\", \"ecmerge\", \"tortoisemerge\", \"p4merge\", \"araxis\", \"opendiff\"\n+\tand \"diffmerge\".  Any other value is treated is custom merge tool\n \tand there must be a corresponding mergetool.<tool>.cmd option.\n \n merge.verbosity::\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7c18b0c..5cc5ee7 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -975,7 +975,7 @@ _git_diff ()\n }\n \n __git_mergetools_common=\"diffuse ecmerge emerge kdiff3 meld opendiff\n-\t\t\ttkdiff vimdiff gvimdiff xxdiff araxis p4merge\n+\t\t\ttkdiff vimdiff gvimdiff xxdiff araxis p4merge diffmerge\n \"\n \n _git_difftool ()\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 5b62785..5b29fef 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -46,7 +46,8 @@ check_unchanged () {\n valid_tool () {\n \tcase \"$1\" in\n \tkdiff3 | tkdiff | xxdiff | meld | opendiff | \\\n-\temerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge)\n+\temerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge | \\\n+\tdiffmerge)\n \t\t;; # happy\n \ttortoisemerge)\n \t\tif ! merge_mode; then\n@@ -297,6 +298,23 @@ run_merge_tool () {\n \t\t\t\t>/dev/null 2>&1\n \t\tfi\n \t\t;;\n+\tdiffmerge)\n+\t\tif merge_mode; then\n+\t\t\tif $base_present; then\n+\t\t\t\t\"$merge_tool_path\" -nosplash -merge -result=\"$MERGED\" \\\n+\t\t\t\t\t\"$LOCAL\" \"$BASE\" \"$REMOTE\"\n+\t\t\t\t\t>/dev/null 2>&1\n+\t\t\telse\n+\t\t\t\t\"$merge_tool_path\" -nosplash -merge \\\n+\t\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\t\t\t\t>/dev/null 2>&1\n+\t\t\tfi\n+\t\t\tstatus=$?\n+\t\telse\n+\t\t\t\"$merge_tool_path\" -nosplash \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t\t>/dev/null 2>&1\n+\t\tfi\n+\t\t;;\n \t*)\n \t\tmerge_tool_cmd=\"$(get_merge_tool_cmd \"$1\")\"\n \t\tif test -z \"$merge_tool_cmd\"; then\n@@ -336,7 +354,7 @@ guess_merge_tool () {\n \t\telse\n \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n \t\tfi\n-\t\ttools=\"$tools gvimdiff diffuse ecmerge p4merge araxis\"\n+\t\ttools=\"$tools gvimdiff diffuse ecmerge p4merge araxis diffmerge\"\n \tfi\n \tcase \"${VISUAL:-$EDITOR}\" in\n \t*vim*)\n-- \n1.6.6.rc1.296.ge77fc.dirty\n"},{"id":"129666","messageId":"20091209223409.GA32744@hashpling.org","threadId":"21909","inReplyTo":"1260302477-49412-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-12-09T22:34:09Z","receivedAt":"2009-12-09T22:34:09Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Tue, Dec 08, 2009 at 12:01:17PM -0800, Jay Soffian wrote:\n> Add SourceGear DiffMerge to the set of built-in diff/merge tools, and update\n> bash completion and documentation.\n> ---\n>  Documentation/git-difftool.txt         |    2 +-\n>  Documentation/git-mergetool.txt        |    2 +-\n>  Documentation/merge-config.txt         |    4 ++--\n>  contrib/completion/git-completion.bash |    2 +-\n>  git-mergetool--lib.sh                  |   22 ++++++++++++++++++++--\n>  5 files changed, 25 insertions(+), 7 deletions(-)\n\nI'm not a diffmerge user but the patch looks fine to me.\n\nIs diffmerge free but not Free?\n\nAny reason for holding back on sign-off?\n\nCharles.\n"},{"id":"129668","messageId":"76718490912091514l16f6dd71g3f644ccf4ef5835c@mail.gmail.com","threadId":"21909","inReplyTo":"20091209223409.GA32744@hashpling.org","subject":"Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-12-09T23:14:23Z","receivedAt":"2009-12-09T23:14:23Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Dec 9, 2009 at 2:34 PM, Charles Bailey <charles@hashpling.org> wrote:\n> Is diffmerge free but not Free?\n\nCorrect, it's free as in beer, not as in speech. It is multi-platform\nthough (Linux, Windows, OS X). I've tested only on MacOS X. If someone\nelse wants to give it a test on Linux, that would be good. Looks like\nthere's both a deb and an rpm:\n\nhttp://www.sourcegear.com/diffmerge/downloads.html\n\n> Any reason for holding back on sign-off?\n\nOversight:\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n\nj.\n"},{"id":"129669","messageId":"7vaaxrn10o.fsf@alter.siamese.dyndns.org","threadId":"21909","inReplyTo":"1260302477-49412-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-10T00:08:55Z","receivedAt":"2009-12-10T00:08:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index 5b62785..5b29fef 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -46,7 +46,8 @@ check_unchanged () {\n>  valid_tool () {\n>  \tcase \"$1\" in\n>  \tkdiff3 | tkdiff | xxdiff | meld | opendiff | \\\n> -\temerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge)\n> +\temerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge | \\\n> +\tdiffmerge)\n>  \t\t;; # happy\n>  \ttortoisemerge)\n>  \t\tif ! merge_mode; then\n\nAs we are in pre-release feature freeze, it doesn't matter very much if I\ntake this patch now, so I'll let it sit in the list archive for now.\n\nBut I have to wonder about the maintainability of this file, if we have to\nadd every time somebody finds yet another diff/merge backends that could\nbe used, even a closed-source one.\n\nThere are only a handful of entry points that mergetool--lib defines, and\nby overriding what should happen when these entry points are called, an end\nuser should be able to tell mergetool/difftool to use a new backend.\n\nPerhaps it is a better approach to first eject bulk of code for the\nbackends we currently support under these case statements into separate\nfiles, one per backend, move them to mergetool/ subdirectory in the source\ntree, install them as \"$(share)/git-core/mergetool/$toolname\", and at\nruntime source them?  That way, a patch to add a new backend can be as\nsimple as adding a new file in mergetool/ and doing nothing else.  Also an\nend user can privately add support to a new backend much more easily.\n\nAnybody want to try that approach?\n"},{"id":"129670","messageId":"76718490912091616n250a4b93h941c91b572c5be1c@mail.gmail.com","threadId":"21909","inReplyTo":"7vaaxrn10o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-12-10T00:16:39Z","receivedAt":"2009-12-10T00:16:39Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Dec 9, 2009 at 4:08 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Perhaps it is a better approach to first eject bulk of code for the\n> backends we currently support under these case statements into separate\n> files, one per backend, move them to mergetool/ subdirectory in the source\n> tree, install them as \"$(share)/git-core/mergetool/$toolname\", and at\n> runtime source them?  That way, a patch to add a new backend can be as\n> simple as adding a new file in mergetool/ and doing nothing else.  Also an\n> end user can privately add support to a new backend much more easily.\n>\n> Anybody want to try that approach?\n\nSure. There may also be value in emulating the mercurial approach:\n\n  http://mercurial.selenic.com/wiki/MergeToolConfiguration\n\nI'll give both a shot.\n\nj.\n"},{"id":"173592","messageId":"20110816100949.GA10859@gmail.com","threadId":"21909","inReplyTo":"7vaaxrn10o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-16T10:09:52Z","receivedAt":"2011-08-16T10:09:52Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"\n(continuing an old thread...)\nhttp://thread.gmane.org/gmane.comp.version-control.git/134906/focus=135006\n\nOn Wed, Dec 09, 2009 at 04:08:55PM -0800, Junio C Hamano wrote:\n> Jay Soffian <jaysoffian@gmail.com> writes:\n> \n> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> > index 5b62785..5b29fef 100644\n> > --- a/git-mergetool--lib.sh\n> > +++ b/git-mergetool--lib.sh\n> > @@ -46,7 +46,8 @@ check_unchanged () {\n> >  valid_tool () {\n> >  \tcase \"$1\" in\n> >  \tkdiff3 | tkdiff | xxdiff | meld | opendiff | \\\n> > -\temerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge)\n> > +\temerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge | \\\n> > +\tdiffmerge)\n> >  \t\t;; # happy\n> >  \ttortoisemerge)\n> >  \t\tif ! merge_mode; then\n> \n> As we are in pre-release feature freeze, it doesn't matter very much if I\n> take this patch now, so I'll let it sit in the list archive for now.\n> \n> But I have to wonder about the maintainability of this file, if we have to\n> add every time somebody finds yet another diff/merge backends that could\n> be used, even a closed-source one.\n> \n> There are only a handful of entry points that mergetool--lib defines, and\n> by overriding what should happen when these entry points are called, an end\n> user should be able to tell mergetool/difftool to use a new backend.\n> \n> Perhaps it is a better approach to first eject bulk of code for the\n> backends we currently support under these case statements into separate\n> files, one per backend, move them to mergetool/ subdirectory in the source\n> tree, install them as \"$(share)/git-core/mergetool/$toolname\", and at\n> runtime source them?  That way, a patch to add a new backend can be as\n> simple as adding a new file in mergetool/ and doing nothing else.  Also an\n> end user can privately add support to a new backend much more easily.\n> \n> Anybody want to try that approach?\n\nI've started working on splitting this file apart and it is\ncoming along nicely.  I should have some patches soon.\n\nWe're approaching feature freeze so I will hold onto them for a\nbit.  I worked the \"meld should use --output with >= 1.5.0\"\ncheck into it too, which worked out nicely with the refactored\nsetup as that logic is neatly tucked away into the meld file.\n\nIs it okay if we install the files into\n$(git --exec-path)/mergetools/ instead?  I didn't spot an\nobvious way to construct $(share)/git-core/ at runtime\n(without dirname tricks that assume share=$(prefix)/share).\n-- \n\t\t\t\t\tDavid\n"},{"id":"173623","messageId":"7v8vqti763.fsf@alter.siamese.dyndns.org","threadId":"21909","inReplyTo":"20110816100949.GA10859@gmail.com","subject":"Re: [PATCH] mergetool--lib: add diffmerge as a pre-configured mergetool option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-16T17:43:16Z","receivedAt":"2011-08-16T17:43:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> Is it okay if we install the files into\n> $(git --exec-path)/mergetools/ instead?\n\nSurely, and thanks.\n\nIt is much better than \"$(git --exec-path)/mergetools--$backend\" ;-).\n"}]}