{"thread":{"id":"7295","subject":"[PATCH] mergetool: Use merge.tool config option.","startedAt":"2007-03-18T16:13:11Z","lastAt":"2007-03-19T04:09:29Z","messageCount":5,"participants":["James Bowes","Junio C Hamano","Theodore Tso"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"37382","messageId":"11742343911678-git-send-email-jbowes@dangerouslyinc.com","threadId":"7295","inReplyTo":null,"subject":"[PATCH] mergetool: Use merge.tool config option.","fromName":"James Bowes","fromEmail":"jbowes@dangerouslyinc.com","sentAt":"2007-03-18T16:13:11Z","receivedAt":"2007-03-18T16:13:11Z","isPatch":true,"sender":{"key":"jbowes@dangerouslyinc.com","avatar":"https://gravatar.com/avatar/a2fe98c66b2b47a9fa9d2ba92ff949d54c3208b1f8acc2e745b4b84ae3c4483a?d=mp&s=160"},"body":"If no merge program was supplied on the commandline, and the config option\nmerge.tool was set to a valid value, then mergetool would unset $merge_tool\nand instead try to find an installed merge program. This patch removes the code\nthat unset $merge_tool, so the merge.tool config option will always be used, if\nset.\n\nSigned-off-by: James Bowes \n---\n git-mergetool.sh | 4 ----\n 1 files changed, 0 insertions(+), 4 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 52386a5..19788a1 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -288,10 +288,6 @@ done\n \n if test -z \"$merge_tool\"; then\n merge_tool=`git-config merge.tool`\n- if test $merge_tool = kdiff3 -o $merge_tool = tkdiff -o \\\n-\t$merge_tool = xxdiff -o $merge_tool = meld ; then\n-\tunset merge_tool\n- fi\n fi\n \n if test -z \"$merge_tool\" ; then\n-- \n1.5.0.3\n"},{"id":"37431","messageId":"7vwt1em6gf.fsf@assigned-by-dhcp.cox.net","threadId":"7295","inReplyTo":"11742343911678-git-send-email-jbowes@dangerouslyinc.com","subject":"Re: [PATCH] mergetool: Use merge.tool config option.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-19T00:18:24Z","receivedAt":"2007-03-19T00:18:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"James Bowes writes:\n\n> If no merge program was supplied on the commandline, and the config option\n> merge.tool was set to a valid value, then mergetool would unset $merge_tool\n> and instead try to find an installed merge program. This patch removes the code\n> that unset $merge_tool, so the merge.tool config option will always be used, if\n> set.\n\nThe problem description looks correct, but I think the original\nmeant to reject configuration value for merge_tool that is not\nsupported with the version of the script (and screwed up).\n\n> Signed-off-by: James Bowes \n> ---\n> git-mergetool.sh | 4 ----\n> 1 files changed, 0 insertions(+), 4 deletions(-)\n>\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 52386a5..19788a1 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -288,10 +288,6 @@ done\n> \n> if test -z \"$merge_tool\"; then\n> merge_tool=`git-config merge.tool`\n> - if test $merge_tool = kdiff3 -o $merge_tool = tkdiff -o \\\n> -\t$merge_tool = xxdiff -o $merge_tool = meld ; then\n> -\tunset merge_tool\n> - fi\n> fi\n> \n> if test -z \"$merge_tool\" ; then\n\nIOW, wouldn't this be a better way?\n\n if test -z \"$merge_tool\"\n then\n merge_tool=`git-config merge.tool`\n case \"$merge_tool\" in\n kdiff3 | tkdiff | xxdiff | meld | emerge)\n ;; # happy\n *)\n echo >&2 \"We do not know how to drive $merge_tool\"\n echo >&2 \"Resetting to default...\"\n unset merge_tool\n ;;\n esac\n fi\n"},{"id":"37432","messageId":"3f80363f0703181811x54acb3f4n689f4fd68f5a5dbe@mail.gmail.com","threadId":"7295","inReplyTo":"7vwt1em6gf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] mergetool: Use merge.tool config option.","fromName":"James Bowes","fromEmail":"jbowes@dangerouslyinc.com","sentAt":"2007-03-19T01:11:07Z","receivedAt":"2007-03-19T01:11:07Z","isPatch":true,"sender":{"key":"jbowes@dangerouslyinc.com","avatar":"https://gravatar.com/avatar/a2fe98c66b2b47a9fa9d2ba92ff949d54c3208b1f8acc2e745b4b84ae3c4483a?d=mp&s=160"},"body":"On 3/18/07, Junio C Hamano wrote:\n> The problem description looks correct, but I think the original\n> meant to reject configuration value for merge_tool that is not\n> supported with the version of the script (and screwed up).\n\nThere's a bit later on in mergetool that errors out if you have\nprovided an unknown merge program (either via the command line or\nthrough your config). The command line and the config ways should\nprobably behave the same, eh? If so, the case block should be brought\nup one level like so:\n\n> IOW, wouldn't this be a better way?\n>\n> if test -z \"$merge_tool\"\n> then\n> merge_tool=`git-config merge.tool`\n fi\n case \"$merge_tool\" in\n kdiff3 | tkdiff | xxdiff | meld | emerge)\n ;; # happy\n *)\n echo >&2 \"We do not know how to drive $merge_tool\"\n echo >&2 \"Resetting to default...\"\n unset merge_tool\n ;;\n esac\n\nAnd then remove the 'Unknown mergetool' bit.\n\nI think either way is fine since they both let you know that you've\nentered gobbledeegook or forgot to install something, so I'll defer to\nyou all for the choice on which way to go.\n\n-James\n"},{"id":"37443","messageId":"20070319023238.GC11371@thunk.org","threadId":"7295","inReplyTo":"3f80363f0703181811x54acb3f4n689f4fd68f5a5dbe@mail.gmail.com","subject":"Re: [PATCH] mergetool: Use merge.tool config option.","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2007-03-19T02:32:38Z","receivedAt":"2007-03-19T02:32:38Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"It seemed to me that Junio's suggestion made the most amount of sense,\nbut I tinkered with the with the warning message to make it clear that\nthe cause of the warning was a bugus tool in the merge.tool\nconfiguration parameter.\n\nThis has also been pushed out to git://repo.or.cz/git/mergetool.git\n\nJunio, please pull if you approve...\n\n\t\t\t\t\t- Ted\n\ncommit d6678c28e30e836449092a2917d4b0bd6254b06c\nAuthor: Theodore Ts'o \nDate: Sun Mar 18 22:30:10 2007 -0400\n\n mergetool: print an appropriate warning if merge.tool is unknown\n \n Also add support for vimdiff\n \n Signed-off-by: \"Theodore Ts'o\" \n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 563c5c0..7942fd0 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -288,10 +288,15 @@ done\n \n if test -z \"$merge_tool\"; then\n merge_tool=`git-config merge.tool`\n- if test $merge_tool = kdiff3 -o $merge_tool = tkdiff -o \\\n-\t$merge_tool = xxdiff -o $merge_tool = meld ; then\n-\tunset merge_tool\n- fi\n+ case \"$merge_tool\" in\n+\tkdiff3 | tkdiff | xxdiff | meld | emerge | vimdiff)\n+\t ;; # happy\n+\t*)\n+\t echo >&2 \"git config option merge.tool set to unknown tool: $merge_tool\"\n+\t echo >&2 \"Resetting to default...\"\n+\t unset merge_tool\n+\t ;;\n+ esac\n fi\n \n if test -z \"$merge_tool\" ; then\n"},{"id":"37462","messageId":"7vircxnabq.fsf@assigned-by-dhcp.cox.net","threadId":"7295","inReplyTo":"20070319023238.GC11371@thunk.org","subject":"Re: [PATCH] mergetool: Use merge.tool config option.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-19T04:09:29Z","receivedAt":"2007-03-19T04:09:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Theodore Tso writes:\n\n> It seemed to me that Junio's suggestion made the most amount of sense,\n> but I tinkered with the with the warning message to make it clear that\n> the cause of the warning was a bugus tool in the merge.tool\n> configuration parameter.\n>\n> This has also been pushed out to git://repo.or.cz/git/mergetool.git\n>\n> Junio, please pull if you approve...\n\nSurely, and thanks. I think your message is much nicer.\n"}]}