{"thread":{"id":"32756","subject":"[PATCH] fixup! mergetools: simplify how we handle \"vim\" and \"defaults\"","startedAt":"2013-01-28T19:28:56Z","lastAt":"2013-01-29T03:02:47Z","messageCount":3,"participants":["John Keeping","David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"208108","messageId":"20130128192856.GA7498@serenity.lan","threadId":"32756","inReplyTo":null,"subject":"[PATCH] fixup! mergetools: simplify how we handle \"vim\" and \"defaults\"","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-01-28T19:28:56Z","receivedAt":"2013-01-28T19:28:56Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"---\nJunio, please can you squash this into f9924e5 on jk/mergetool,\nproviding that David is OK with that?\n\nThe original change breaks custom mergetool by making changing the logic\naround default functions so that they are now only defined when the tool\nfile exists in $MERGE_TOOLS_DIR but we need the default implementations\nwhen a custom tool is in use, which by definition means that the file\ndoesn't exist in $MERGE_TOOLS_DIR.\n\n git-mergetool--lib.sh | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 1d0fb12..211ffe5 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -48,15 +48,6 @@ valid_tool () {\n setup_tool () {\n \ttool=\"$1\"\n \n-\tif ! test -f \"$MERGE_TOOLS_DIR/$tool\"\n-\tthen\n-\t\t# Use a special return code for this case since we want to\n-\t\t# source \"defaults\" even when an explicit tool path is\n-\t\t# configured since the user can use that to override the\n-\t\t# default path in the scriptlet.\n-\t\treturn 2\n-\tfi\n-\n \t# Fallback definitions, to be overriden by tools.\n \tcan_merge () {\n \t\treturn 0\n@@ -80,6 +71,15 @@ setup_tool () {\n \t\techo \"$1\"\n \t}\n \n+\tif ! test -f \"$MERGE_TOOLS_DIR/$tool\"\n+\tthen\n+\t\t# Use a special return code for this case since we want to\n+\t\t# source \"defaults\" even when an explicit tool path is\n+\t\t# configured since the user can use that to override the\n+\t\t# default path in the scriptlet.\n+\t\treturn 2\n+\tfi\n+\n \t# Load the redefined functions\n \t. \"$MERGE_TOOLS_DIR/$tool\"\n \n-- \n1.8.1.1\n"},{"id":"208142","messageId":"CAJDDKr5wsYqnBk+kvKj1qzUYKJBJ5nAiwMJZ=HpzPPgJf8BBTQ@mail.gmail.com","threadId":"32756","inReplyTo":"20130128192856.GA7498@serenity.lan","subject":"Re: [PATCH] fixup! mergetools: simplify how we handle \"vim\" and \"defaults\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-01-29T01:37:13Z","receivedAt":"2013-01-29T01:37:13Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, Jan 28, 2013 at 11:28 AM, John Keeping <john@keeping.me.uk> wrote:\n> ---\n> Junio, please can you squash this into f9924e5 on jk/mergetool,\n> providing that David is OK with that?\n\nThis looks good to me.\n\n> The original change breaks custom mergetool by making changing the logic\n> around default functions so that they are now only defined when the tool\n> file exists in $MERGE_TOOLS_DIR but we need the default implementations\n> when a custom tool is in use, which by definition means that the file\n> doesn't exist in $MERGE_TOOLS_DIR.\n>\n>  git-mergetool--lib.sh | 18 +++++++++---------\n>  1 file changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index 1d0fb12..211ffe5 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -48,15 +48,6 @@ valid_tool () {\n>  setup_tool () {\n>         tool=\"$1\"\n>\n> -       if ! test -f \"$MERGE_TOOLS_DIR/$tool\"\n> -       then\n> -               # Use a special return code for this case since we want to\n> -               # source \"defaults\" even when an explicit tool path is\n> -               # configured since the user can use that to override the\n> -               # default path in the scriptlet.\n> -               return 2\n> -       fi\n> -\n>         # Fallback definitions, to be overriden by tools.\n>         can_merge () {\n>                 return 0\n> @@ -80,6 +71,15 @@ setup_tool () {\n>                 echo \"$1\"\n>         }\n>\n> +       if ! test -f \"$MERGE_TOOLS_DIR/$tool\"\n> +       then\n> +               # Use a special return code for this case since we want to\n> +               # source \"defaults\" even when an explicit tool path is\n> +               # configured since the user can use that to override the\n> +               # default path in the scriptlet.\n> +               return 2\n> +       fi\n> +\n>         # Load the redefined functions\n>         . \"$MERGE_TOOLS_DIR/$tool\"\n>\n> --\n> 1.8.1.1\n\n\n\n-- \nDavid\n"},{"id":"208143","messageId":"7vy5fcu1wo.fsf@alter.siamese.dyndns.org","threadId":"32756","inReplyTo":"CAJDDKr5wsYqnBk+kvKj1qzUYKJBJ5nAiwMJZ=HpzPPgJf8BBTQ@mail.gmail.com","subject":"Re: [PATCH] fixup! mergetools: simplify how we handle \"vim\" and \"defaults\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-29T03:02:47Z","receivedAt":"2013-01-29T03:02:47Z","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> On Mon, Jan 28, 2013 at 11:28 AM, John Keeping <john@keeping.me.uk> wrote:\n>> ---\n>> Junio, please can you squash this into f9924e5 on jk/mergetool,\n>> providing that David is OK with that?\n>\n> This looks good to me.\n\nThanks for a quick response.  Will squash this in and rebuild the\ntwo affected branches.\n"}]}