{"thread":{"id":"17407","subject":"[PATCH] mergetool merge/skip/abort at prompt","startedAt":"2009-01-28T06:56:47Z","lastAt":"2009-01-28T09:53:12Z","messageCount":6,"participants":["Caleb Cushing","David Aguilar","Charles Bailey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"102256","messageId":"81bfc67a0901272256t726bf206k351bb6c8b2778bd5@mail.gmail.com","threadId":"17407","inReplyTo":null,"subject":"[PATCH] mergetool merge/skip/abort at prompt","fromName":"Caleb Cushing","fromEmail":"xenoterracide@gmail.com","sentAt":"2009-01-28T06:56:47Z","receivedAt":"2009-01-28T06:56:47Z","isPatch":true,"sender":{"key":"xenoterracide@gmail.com","avatar":"https://gravatar.com/avatar/af3f0745dfa0ea9c4ee551d7d0a3cfe7ba8d229754c11678ab2ed23c3fa57065?d=mp&s=160"},"body":"previously git mergetool when run with prompt only allowed the user to continue\nmerging. This changes git mergetool to allow the option of skipping a file or\naborting, and includes an addtional key to explicitly select merge.\n\nSigned-off-by: Caleb Cushing <xenoterracide@gmail.com>\n---\n git-mergetool.sh |   20 ++++++++++++++++++--\n 1 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 00e1337..575fbb2 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -177,8 +177,24 @@ merge_file () {\n     describe_file \"$local_mode\" \"local\" \"$LOCAL\"\n     describe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n     if \"$prompt\" = true; then\n-\tprintf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n-\tread ans\n+\twhile true; do\n+\t    printf \"Use (m)erge file or (s)kip file, or (a)bort? (%s): \" \\\n+\t    \"$merge_tool\"\n+\t    read ans\n+\t    case \"$ans\" in\n+\t\t[mM]*|\"\")\n+\t\t    break\n+\t\t    ;;\n+\t\t[sS]*)\n+\t\t    cleanup_temp_files\n+\t\t    return 0\n+\t\t    ;;\n+\t\t[aA]*)\n+\t\t    cleanup_temp_files\n+\t\t    exit 0\n+\t\t    ;;\n+\t    esac\n+\tdone\n     fi\n\n     case \"$merge_tool\" in\n-- \n1.6.1.1\n"},{"id":"102258","messageId":"81bfc67a0901272301y88162f6xc255195d59765ff@mail.gmail.com","threadId":"17407","inReplyTo":"81bfc67a0901272256t726bf206k351bb6c8b2778bd5@mail.gmail.com","subject":"Re: [PATCH] mergetool merge/skip/abort at prompt","fromName":"Caleb Cushing","fromEmail":"xenoterracide@gmail.com","sentAt":"2009-01-28T07:01:45Z","receivedAt":"2009-01-28T07:01:45Z","isPatch":true,"sender":{"key":"xenoterracide@gmail.com","avatar":"https://gravatar.com/avatar/af3f0745dfa0ea9c4ee551d7d0a3cfe7ba8d229754c11678ab2ed23c3fa57065?d=mp&s=160"},"body":"better? I think I got the formatting fixed... got the imap working...\nadded a more descriptive commit message. hoping that everything looks\ngood.\n"},{"id":"102276","messageId":"402731c90901280004l29382eaanedfdfcca75529468@mail.gmail.com","threadId":"17407","inReplyTo":"81bfc67a0901272256t726bf206k351bb6c8b2778bd5@mail.gmail.com","subject":"Re: [PATCH] mergetool merge/skip/abort at prompt","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-01-28T08:04:22Z","receivedAt":"2009-01-28T08:04:22Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Hi Caleb\n\nOn Tue, Jan 27, 2009 at 10:56 PM, Caleb Cushing <xenoterracide@gmail.com> wrote:\n> previously git mergetool when run with prompt only allowed the user to continue\n> merging. This changes git mergetool to allow the option of skipping a file or\n> aborting, and includes an addtional key to explicitly select merge.\n>\n> Signed-off-by: Caleb Cushing <xenoterracide@gmail.com>\n> ---\n>  git-mergetool.sh |   20 ++++++++++++++++++--\n>  1 files changed, 18 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 00e1337..575fbb2 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -177,8 +177,24 @@ merge_file () {\n>     describe_file \"$local_mode\" \"local\" \"$LOCAL\"\n>     describe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n>     if \"$prompt\" = true; then\n> -       printf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n> -       read ans\n> +       while true; do\n> +           printf \"Use (m)erge file or (s)kip file, or (a)bort? (%s): \" \\\n\n\nI really like the feature you added here.  I'm sorry to bikeshed on\nthis conversation, but after trying it I have one tiny suggestion.\n\nRight now the prompt looks like this with your patch:\n\n\"\"\"\nMerging the files: foo bar baz\n\nNormal merge conflict for 'foo':\n  {local}: created\n  {remote}: created\nUse (m)erge file or (s)kip file, or (a)bort? (xxdiff): m\n\"\"\"\n\ndo you think\n\"Use merge file or skip file, or abort?\"\nmight be better expressed as:\n\n\"\"\"\nMerging: foo bar baz\n\nNormal merge conflict for 'foo':\n  {local}: created\n  {remote}: created\n(m)erge, (s)kip, or (q)uit? (xxdiff): m\n\"\"\"?\n\nI realize that your patch only touches the last line of the prompt\n(and not the introductory \"Merging the files:\" line) so if you agree\nthen maybe I can throw a patch together for the introductory line.\n\nAlso, my example has quit instead of abort for two reasons (the first\none is silly)\n1. skip rhymes with quit, so it reads very nicely out loud\n2. consistency with git add --interactive\n3. less typos (q and s are diagonal on qwerty, s and a are adjacent)\n(okay, that last one is silly too)\n\nSome might also mis-associate 'abort' with meaning \"abort the merge.\"\n\nslightly off-topic:\nIf we're looking at cleaning up mergetool a bit would you all mind a\nseparate patch to convert it to using hard tabs throughout, just like\ngit-rebase.sh?\n\n\n> +           \"$merge_tool\"\n> +           read ans\n> +           case \"$ans\" in\n> +               [mM]*|\"\")\n> +                   break\n> +                   ;;\n> +               [sS]*)\n> +                   cleanup_temp_files\n> +                   return 0\n> +                   ;;\n> +               [aA]*)\n> +                   cleanup_temp_files\n> +                   exit 0\n> +                   ;;\n> +           esac\n> +       done\n>     fi\n>\n>     case \"$merge_tool\" in\n> --\n> 1.6.1.1\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n\n\n\n-- \n    David\n"},{"id":"102283","messageId":"20090128084756.GA28493@hashpling.org","threadId":"17407","inReplyTo":"81bfc67a0901272256t726bf206k351bb6c8b2778bd5@mail.gmail.com","subject":"Re: [PATCH] mergetool merge/skip/abort at prompt","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-01-28T08:47:56Z","receivedAt":"2009-01-28T08:47:56Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Wed, Jan 28, 2009 at 01:56:47AM -0500, Caleb Cushing wrote:\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 00e1337..575fbb2 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -177,8 +177,24 @@ merge_file () {\n>      describe_file \"$local_mode\" \"local\" \"$LOCAL\"\n>      describe_file \"$remote_mode\" \"remote\" \"$REMOTE\"\n>      if \"$prompt\" = true; then\n> -\tprintf \"Hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n> -\tread ans\n> +\twhile true; do\n> +\t    printf \"Use (m)erge file or (s)kip file, or (a)bort? (%s): \" \\\n> +\t    \"$merge_tool\"\n> +\t    read ans\n> +\t    case \"$ans\" in\n> +\t\t[mM]*|\"\")\n> +\t\t    break\n> +\t\t    ;;\n> +\t\t[sS]*)\n> +\t\t    cleanup_temp_files\n> +\t\t    return 0\n> +\t\t    ;;\n> +\t\t[aA]*)\n> +\t\t    cleanup_temp_files\n> +\t\t    exit 0\n> +\t\t    ;;\n> +\t    esac\n> +\tdone\n\nThis patch does now apply for me, so I've given it a longer look. It\ndoes roughly what I expect, but I can't help feeling that the change\nisn't in the best place.\n\nCurrently, whatever the prompt, the merge tool will always be run so\nit makes sense (or at least there is no negative) in creating the\ntemporary files before running the merge tool.\n\nWith this change, it would seem to be more logical to ask whether the\nmerge tool is to be run before creating the temporary files, removing\nthe need for them to be cleaned up if the answer is no. I think that\nthis would be cleaner overall.\n\nAt the same time, however, it might be worth refactoring the\nmerge_file function as the same criticism could probably levelled at\nthe code paths that perform symlink and deleted file merges and these\npaths would probably now share much more of the logic and behaviour of\na normal file merge.\n\nTrying out this refactoring and adding the option to choose local or\nremote file versions without running the merge tool has been on my\ntodo list for a while, but I might actually have a go at it this\nweekend if nobody beats me to it.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"102289","messageId":"81bfc67a0901280150i799c881fyf3f506d7ba3c26d6@mail.gmail.com","threadId":"17407","inReplyTo":"402731c90901280004l29382eaanedfdfcca75529468@mail.gmail.com","subject":"Re: [PATCH] mergetool merge/skip/abort at prompt","fromName":"Caleb Cushing","fromEmail":"xenoterracide@gmail.com","sentAt":"2009-01-28T09:50:03Z","receivedAt":"2009-01-28T09:50:03Z","isPatch":true,"sender":{"key":"xenoterracide@gmail.com","avatar":"https://gravatar.com/avatar/af3f0745dfa0ea9c4ee551d7d0a3cfe7ba8d229754c11678ab2ed23c3fa57065?d=mp&s=160"},"body":">  Also, my example has quit instead of abort for two reasons (the first\n>  one is silly)\n>  1. skip rhymes with quit, so it reads very nicely out loud\n>  2. consistency with git add --interactive\n>  3. less typos (q and s are diagonal on qwerty, s and a are adjacent)\n>  (okay, that last one is silly too)\n\nI chose abort because it's used in other places in mergetool (for same\npurpose). I'm not opposed to cleaning up or making it more consistent\nwith other utilities though. but perhaps that's for another patch...\n\n>  slightly off-topic:\n>  If we're looking at cleaning up mergetool a bit would you all mind a\n>  separate patch to convert it to using hard tabs throughout, just like\n>  git-rebase.sh?\n\nin an earlier thread... I complained loudly about the mixing of tabs\nand spaces, it should be banned imho, causes nothing but problems.\n\n-- \nCaleb Cushing\n\nhttp://xenoterracide.blogspot.com\n"},{"id":"102291","messageId":"81bfc67a0901280153v33993d73p63687c78df555b48@mail.gmail.com","threadId":"17407","inReplyTo":"20090128084756.GA28493@hashpling.org","subject":"Re: [PATCH] mergetool merge/skip/abort at prompt","fromName":"Caleb Cushing","fromEmail":"xenoterracide@gmail.com","sentAt":"2009-01-28T09:53:12Z","receivedAt":"2009-01-28T09:53:12Z","isPatch":true,"sender":{"key":"xenoterracide@gmail.com","avatar":"https://gravatar.com/avatar/af3f0745dfa0ea9c4ee551d7d0a3cfe7ba8d229754c11678ab2ed23c3fa57065?d=mp&s=160"},"body":">  With this change, it would seem to be more logical to ask whether the\n>  merge tool is to be run before creating the temporary files, removing\n>  the need for them to be cleaned up if the answer is no. I think that\n>  this would be cleaner overall.\n\nI agree, but to be honest, I couldn't get the logic wrapped around my\nhead, so I did it this way. refactoring does seem to be the best idea,\nbut I don't understand enough of it yet to do so.\n\n-- \nCaleb Cushing\n\nhttp://xenoterracide.blogspot.com\n"}]}