{"thread":{"id":"17611","subject":"[PATCH] Offer to print changes while running git-mergetool","startedAt":"2009-02-06T14:32:25Z","lastAt":"2009-02-08T12:38:55Z","messageCount":10,"participants":["Jonathan del Strother","Junio C Hamano","Charles Bailey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103487","messageId":"1233930745-77930-1-git-send-email-jon.delStrother@bestbefore.tv","threadId":"17611","inReplyTo":null,"subject":"[PATCH] Offer to print changes while running git-mergetool","fromName":"Jonathan del Strother","fromEmail":"jon.delstrother@bestbefore.tv","sentAt":"2009-02-06T14:32:25Z","receivedAt":"2009-02-06T14:32:25Z","isPatch":true,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"Add a \"Show changes\" option to each prompt in mergetool. This prints the conflicted changes on the current file, using 'git log -p --merge '\n\nSigned-off-by: Jonathan del Strother \n---\n\nI frequently find myself running git-mergetool, then finding halfway through that I need to review the changes that produced that conflict.\nHow about something like this, for showing the changes from within mergetool?\n\n git-mergetool.sh | 37 ++++++++++++++++++++++++++++++-------\n 1 files changed, 30 insertions(+), 7 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 87fa88a..9df91d8 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -62,7 +62,7 @@ describe_file () {\n \n resolve_symlink_merge () {\n while true; do\n-\tprintf \"Use (l)ocal or (r)emote, or (a)bort? \"\n+\tprintf \"Use (l)ocal or (r)emote, (s)how changes, or (a)bort? \"\n \tread ans\n \tcase \"$ans\" in\n \t [lL]*)\n@@ -77,6 +77,12 @@ resolve_symlink_merge () {\n \t\tcleanup_temp_files --save-backup\n \t\treturn 0\n \t\t;;\n+\t\t[sS]*)\n+\t\tgit log -p --merge \"$MERGED\"\n+\t\tprintf \"\\n\"\n+\t\tresolve_symlink_merge\n+\t\treturn\n+\t\t;;\n \t [aA]*)\n \t\treturn 1\n \t\t;;\n@@ -87,9 +93,9 @@ resolve_symlink_merge () {\n resolve_deleted_merge () {\n while true; do\n \tif base_present; then\n-\t printf \"Use (m)odified or (d)eleted file, or (a)bort? \"\n+\t printf \"Use (m)odified or (d)eleted file, (s)how changes, or (a)bort? \"\n \telse\n-\t printf \"Use (c)reated or (d)eleted file, or (a)bort? \"\n+\t printf \"Use (c)reated or (d)eleted file, (s)how changes, or (a)bort? \"\n \tfi\n \tread ans\n \tcase \"$ans\" in\n@@ -103,6 +109,12 @@ resolve_deleted_merge () {\n \t\tcleanup_temp_files\n \t\treturn 0\n \t\t;;\n+\t\t[sS]*)\n+\t\tgit log -p --merge \"$MERGED\"\n+\t\tprintf \"\\n\"\n+\t\tresolve_deleted_merge\n+\t\treturn\n+\t\t;;\n \t [aA]*)\n \t\treturn 1\n \t\t;;\n@@ -183,10 +195,21 @@ merge_file () {\n echo \"Normal merge conflict for '$MERGED':\"\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- fi\n+\tif \"$prompt\" = true; then\n+\t\twhile true; do\n+\t\t\tprintf \"(S)how changes, or hit return to start merge resolution tool (%s): \" \"$merge_tool\"\n+\t\t\tread ans\n+\t\t\tcase \"$ans\" in\n+\t\t\t\t[sS]*)\n+\t\t\t\tgit log -p --merge \"$MERGED\"\n+\t\t\t\tprintf \"\\n\"\n+\t\t\t\t;;\n+\t\t\t\t*)\n+\t\t\t\tbreak\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone\n+\tfi\n \n case \"$merge_tool\" in\n \tkdiff3)\n-- \n1.6.1.2.390.gba743.dirty\n"},{"id":"103488","messageId":"57518fd10902060641pa789ffbjceccbf013864e0a5@mail.gmail.com","threadId":"17611","inReplyTo":"1233930745-77930-1-git-send-email-jon.delStrother@bestbefore.tv","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Jonathan del Strother","fromEmail":"maillist@steelskies.com","sentAt":"2009-02-06T14:41:45Z","receivedAt":"2009-02-06T14:41:45Z","isPatch":true,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"On Fri, Feb 6, 2009 at 2:32 PM, Jonathan del Strother\n wrote:\n> Add a \"Show changes\" option to each prompt in mergetool. This prints the conflicted changes on the current file, using 'git log -p --merge '\n\nJust discovered that this doesn't work so well when resolving merges\nresulting from \"git stash apply\" - it produces \"fatal: --merge without\nMERGE_HEAD\". Should git-stash be setting MERGE_HEAD in this case, or\nshould I be using something other than 'git log --merge' to cover\nthese sorts of cases?\n"},{"id":"103534","messageId":"7vocxf5ufu.fsf@gitster.siamese.dyndns.org","threadId":"17611","inReplyTo":"57518fd10902060641pa789ffbjceccbf013864e0a5@mail.gmail.com","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-06T17:47:17Z","receivedAt":"2009-02-06T17:47:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan del Strother writes:\n\n> On Fri, Feb 6, 2009 at 2:32 PM, Jonathan del Strother\n> wrote:\n>> Add a \"Show changes\" option to each prompt in mergetool. This prints the conflicted changes on the current file, using 'git log -p --merge '\n>\n> Just discovered that this doesn't work so well when resolving merges\n> resulting from \"git stash apply\" - it produces \"fatal: --merge without\n> MERGE_HEAD\". Should git-stash be setting MERGE_HEAD in this case,\n\nNo no no, please absolutely don't. MERGE_HEAD is an instruction to the\neventual commit to create a merge commit and use the commits recorded\nthere as other parents when it does so. You do *NOT* want to end up with\na merge with random state after unstashing. None among cherry-pick,\nrebase, checkout -m (branch switching), nor am -3 should.\n\nI'd suggest making the new action conditionally available, by using the\npresense of MERGE_HEAD as a cue.\n\nThe thing is, these commands that can potentially end in conflict operate\nonly at the tree level, and not at the level of commit ancestry graph.\n\"log --merge\" is all about following the commit ancestry graph, and for\nconflicts left by these commands it is not a useful way to review.\n"},{"id":"103542","messageId":"57518fd10902061108k6a7691c5r13b2782baf3bfde3@mail.gmail.com","threadId":"17611","inReplyTo":"7vocxf5ufu.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Jonathan del Strother","fromEmail":"maillist@steelskies.com","sentAt":"2009-02-06T19:08:47Z","receivedAt":"2009-02-06T19:08:47Z","isPatch":true,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"On Fri, Feb 6, 2009 at 5:47 PM, Junio C Hamano wrote:\n> Jonathan del Strother writes:\n>\n>> On Fri, Feb 6, 2009 at 2:32 PM, Jonathan del Strother\n>> wrote:\n>>> Add a \"Show changes\" option to each prompt in mergetool. This prints the conflicted changes on the current file, using 'git log -p --merge '\n>>\n>> Just discovered that this doesn't work so well when resolving merges\n>> resulting from \"git stash apply\" - it produces \"fatal: --merge without\n>> MERGE_HEAD\". Should git-stash be setting MERGE_HEAD in this case,\n>\n> No no no, please absolutely don't. MERGE_HEAD is an instruction to the\n> eventual commit to create a merge commit and use the commits recorded\n> there as other parents when it does so. You do *NOT* want to end up with\n> a merge with random state after unstashing. None among cherry-pick,\n> rebase, checkout -m (branch switching), nor am -3 should.\n>\n> I'd suggest making the new action conditionally available, by using the\n> presense of MERGE_HEAD as a cue.\n>\n> The thing is, these commands that can potentially end in conflict operate\n> only at the tree level, and not at the level of commit ancestry graph.\n> \"log --merge\" is all about following the commit ancestry graph, and for\n> conflicts left by these commands it is not a useful way to review.\n>\n\nMaybe I'm misunderstanding the issue, but it seems like showing the\nconflicted changes somehow could still be useful. eg If I've made\nchanges on branch A, stash, switch to branch B, apply the stash and\nget conflicts, I'd still like to see the commits that produced those\nconflicts.\nObviously for a stash 'merge', one side of the merge isn't that\ninteresting : it's just going to be all the stuff from my working copy\nbefore it was stashed. But wouldn't the other side of the merge (ie\nchanges made on branch B since A & B diverged) produce useful\ninformation?\n\nOff the top of my head, I guess I want something like \"git log -p\n^stash HEAD\", but obviously that doesn't work when dealing with, say,\n\"git stash apply stash@{2}\".\n\nOr is this not workable for some reason I'm not thinking of?\n"},{"id":"103571","messageId":"7veiyb14gr.fsf@gitster.siamese.dyndns.org","threadId":"17611","inReplyTo":"57518fd10902061108k6a7691c5r13b2782baf3bfde3@mail.gmail.com","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T00:21:56Z","receivedAt":"2009-02-07T00:21:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan del Strother writes:\n\n> On Fri, Feb 6, 2009 at 5:47 PM, Junio C Hamano wrote:\n>> Jonathan del Strother writes:\n>>\n>>> On Fri, Feb 6, 2009 at 2:32 PM, Jonathan del Strother\n>>> wrote:\n>>>> Add a \"Show changes\" option to each prompt in mergetool. This prints the conflicted changes on the current file, using 'git log -p --merge '\n>>>\n>>> Just discovered that this doesn't work so well when resolving merges\n>>> resulting from \"git stash apply\" - it produces \"fatal: --merge without\n>>> MERGE_HEAD\". Should git-stash be setting MERGE_HEAD in this case,\n>>\n>> No no no, please absolutely don't. MERGE_HEAD is an instruction to the\n>> eventual commit to create a merge commit and use the commits recorded\n>> there as other parents when it does so. You do *NOT* want to end up with\n>> a merge with random state after unstashing. None among cherry-pick,\n>> rebase, checkout -m (branch switching), nor am -3 should.\n>>\n>> I'd suggest making the new action conditionally available, by using the\n>> presense of MERGE_HEAD as a cue.\n>>\n>> The thing is, these commands that can potentially end in conflict operate\n>> only at the tree level, and not at the level of commit ancestry graph.\n>> \"log --merge\" is all about following the commit ancestry graph, and for\n>> conflicts left by these commands it is not a useful way to review.\n>\n> Maybe I'm misunderstanding the issue,...\n\nThere actually are two independent issues. Sorry for being unclear.\n\nMy objection is about the issue (1) below. I am pointing out (2) merely\nas an issue you need to address if you were to invent a solution that does\nnot have the issue (1); it is not an objection.\n\n(1) Writing MERGE_HEAD from \"git stash apply\" (or \"git stash pop\") is\n absolutely a wrong thing to do.\n\n Typically you use stash this way.\n\n\t... work work work, oops, got interrupted.\n $ git stash save\n ... do some unrelated work.\n ... perhaps switch branches, perhaps not, it does not matter.\n ... the important thing is you conclude this and result in\n ... a clean work tree state.\n $ git stash pop\n ... inspect and resolve issues, perhaps using git log or\n ... git diff or git mergetool\n ... continue to work refining what you were doing\n $ git commit\n\n Imagine what the last \"git commit\" does, IF your \"stash pop\" wrote\n any commit object name in MERGE_HEAD. It will record the tree created\n from the index in a commit that is a merge between the current HEAD\n and whatever commit you wrote in MERGE_HEAD. But you obviously did\n not want any merge --- you wanted a straight and honest single parent\n commit.\n\n It is Ok if your design is to come up with a marker that is different\n from MERGE_HEAD for \"git stash pop\" to tell \"git log --merge\" where\n the potential conflicts may be coming from, but using MERGE_HEAD as\n that marker is unacceptable for this reason.\n\n(2) The set of commits on the left and right side of \"git log --merge\"\n is tied very closely to the topology over the three commits:\n\n $(git merge-base HEAD MERGE_HEAD), which is the base,\n HEAD, which is the right-hand side, and\n MERGE_HEAD, which is the left-hand side.\n\n During a normal merge resolution, \"git log --merge $paths...\" looks at\n the topology of this graph:\n\n x---x---MERGE_HEAD--??? merge result?\n / /\n\tMB---o---o---o---HEAD\n\n and it simply runs\n\n\tgit log MERGE_HEAD...HEAD -- $paths\n\n Hence, x would appear on the left hand side and o on the right hand\n side in the resulting traversal (this makes difference especially if\n you did \"git log --left-right --merge\").\n\nBut \"stash pop\" and \"cherry-pick\" (they are the same thing) work on a\nquite different topology.\n\nIf you stashed while on 'master' (whose tip is A), \"git stash save\" will\ncreate commit S without moving the tip of 'master' (S is saved at the tip\nof refs/stash):\n\n x---x---B topic\n /\n o---o---o---A master\n .\n ...S\n\nWhen you unstash it on B, \"git stash pop\" does a three way merge between A\nand S and B, but it does *not* use the usual ancestry topology.\n\nIt merges S on top of B as if A is the common ancestor (i.e. merge base).\nIf you actually commited S on the master and cherry-picked it on B,\nexactly the same thing happens.\n\nBecause of the way the cherry-picking 3-way merge has to happen, the\n\"virtual ancestory chain\" becomes like this:\n\n *---*---*---x---x---B\n / \\\n A master \\ \n . \\\n ...S..................??? merge result?\n\nwhere * are \"anti-commits\" that reverse the effects of the commits o in\nthe original history that lead to A [*2*].\n\nFirst of all, \"git log A...B -- $path\" won't show the virtual topology\ndepicted above. It will only show commits x and would ignore o, hence it\ndoes not explain the conflicts at all [*1*]. You somehow need to devise a\nway to pretend that the topology is like the above one to achieve an\noutput equivalent to \"git log --merge\" for a true merge situation.\n\nIt is not just the matter of \"echo A >.git/MERGE_HEAD\" (which is an\nabsolute no-no for totally unrelated reason, which is (1) above) to tell\n\"git log --merge\" to pretend the topology is like the above when it does\nits traversal.\n\n\"git log -p A...B -- $path\" won't show them without such a change either,\nbut even if you somehow convinced the revision traversal to pretend the\nthree commits o before A should be shown, you also would need to teach it\nto show them in reverse because they are now anti-commits.\n\nAs I said, the longer explanation of issue (2) in this message is not\nmeant as an objection. I am explaining why \"git log --merge [-p]\" cannot\nbe used as-is for what you are trying to do.\n\nMy primary objection is \"don't mess with MERGE_HEAD\", which is the issue\n(1) above.\n\n\n[Footnote]\n\n*1* you handwaved this issue away in your message saying that what you did\nis not interesting, but I think that is a cop-out. Why are we showing our\nside when inspecting the history in a true merge situation if it is really\nuninteresting?\n\n*2* In addition, there is this little problem that you can cherry-pick\n(and unstash) between disjoint histories, in which case there won't even\nbe such a virtual ancestry chain that I obtained by rotating the history a\nbit in the above example.\n"},{"id":"103598","messageId":"7vr62ay8dh.fsf@gitster.siamese.dyndns.org","threadId":"17611","inReplyTo":"1233930745-77930-1-git-send-email-jon.delStrother@bestbefore.tv","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T08:11:06Z","receivedAt":"2009-02-07T08:11:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan del Strother writes:\n\n> Add a \"Show changes\" option to each prompt in mergetool. This prints the\n> conflicted changes on the current file, using 'git log -p --merge\n> '\n\nI think the patch should look like this, given the recent conversation I\nhad with you. It seems that the script thinks the unit of indentation is\n4-places, and case arms are indented from case/esac (neither of which is\nthe standard git shell script convention), and I tried to match that style\nused in the existing code.\n\nNo, I didn't test it.\n\nCharles volunteered to take over mergetool, so he is on the Cc: list.\n\n git-mergetool.sh | 60 ++++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 47 insertions(+), 13 deletions(-)\n\ndiff --git c/git-mergetool.sh w/git-mergetool.sh\nindex 87fa88a..b8604d6 100755\n--- c/git-mergetool.sh\n+++ w/git-mergetool.sh\n@@ -14,6 +14,13 @@ OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n \n+if test -f \"$GIT_DIR/MERGE_HEAD\"\n+then\n+ in_merge=t show_changes=\", (s)how changes\"\n+else\n+ in_merge=f show_changes=\n+fi\n+\n # Returns true if the mode reflects a symlink\n is_symlink () {\n test \"$1\" = 120000\n@@ -62,22 +69,28 @@ describe_file () {\n \n resolve_symlink_merge () {\n while true; do\n-\tprintf \"Use (l)ocal or (r)emote, or (a)bort? \"\n+\tprintf \"Use (l)ocal or (r)emote$show_changes, or (a)bort? \"\n \tread ans\n-\tcase \"$ans\" in\n-\t [lL]*)\n+\tcase \"$in_merge,$ans\" in\n+\t ?,[lL]*)\n \t\tgit checkout-index -f --stage=2 -- \"$MERGED\"\n \t\tgit add -- \"$MERGED\"\n \t\tcleanup_temp_files --save-backup\n \t\treturn 0\n \t\t;;\n-\t [rR]*)\n+\t ?,[rR]*)\n \t\tgit checkout-index -f --stage=3 -- \"$MERGED\"\n \t\tgit add -- \"$MERGED\"\n \t\tcleanup_temp_files --save-backup\n \t\treturn 0\n \t\t;;\n-\t [aA]*)\n+\t t,[sS]*)\n+\t\tgit log -p --merge \"$MERGED\"\n+\t\tprintf \"\\n\"\n+\t\tresolve_symlink_merge\n+\t\treturn\n+\t\t;;\n+\t ?,[aA]*)\n \t\treturn 1\n \t\t;;\n \t esac\n@@ -87,23 +100,29 @@ resolve_symlink_merge () {\n resolve_deleted_merge () {\n while true; do\n \tif base_present; then\n-\t printf \"Use (m)odified or (d)eleted file, or (a)bort? \"\n+\t printf \"Use (m)odified or (d)eleted file$show_changes, or (a)bort? \"\n \telse\n-\t printf \"Use (c)reated or (d)eleted file, or (a)bort? \"\n+\t printf \"Use (c)reated or (d)eleted file$show changes, or (a)bort? \"\n \tfi\n \tread ans\n-\tcase \"$ans\" in\n-\t [mMcC]*)\n+\tcase \"$in_merge,$ans\" in\n+\t ?,[mMcC]*)\n \t\tgit add -- \"$MERGED\"\n \t\tcleanup_temp_files --save-backup\n \t\treturn 0\n \t\t;;\n-\t [dD]*)\n+\t ?,[dD]*)\n \t\tgit rm -- \"$MERGED\" > /dev/null\n \t\tcleanup_temp_files\n \t\treturn 0\n \t\t;;\n-\t [aA]*)\n+\t t,[sS]*)\n+\t\tgit log -p --merge \"$MERGED\"\n+\t\tprintf \"\\n\"\n+\t\tresolve_deleted_merge\n+\t\treturn\n+\t\t;;\n+\t ?,[aA]*)\n \t\treturn 1\n \t\t;;\n \t esac\n@@ -184,8 +203,23 @@ 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 case $in_merge in\n+\t\tt)\tmsg_head=\"(S)how changes, or h\" ;;\n+\t\tf)\tmsg_head=\"H\" ;;\n+\t esac\n+\t print \"${msg_head}it return to start merge resolution tool (%s): \" \"$merge_tool\"\n+\t read ans\n+\t case \"$in_merge,$ans\" in\n+\t t,[sS]*)\n+\t\t git log -p --merge \"$MERGED\"\n+\t\t printf \"\\n\"\n+\t\t ;;\n+\t\t?,*)\n+\t\t break\n+\t\t ;;\n+\t esac\n+ done\n fi\n \n case \"$merge_tool\" in\n"},{"id":"103613","messageId":"57518fd10902070401x14cc7cacrfb8bc88bbf2999cd@mail.gmail.com","threadId":"17611","inReplyTo":"7vr62ay8dh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Jonathan del Strother","fromEmail":"jon.delstrother@bestbefore.tv","sentAt":"2009-02-07T12:01:37Z","receivedAt":"2009-02-07T12:01:37Z","isPatch":true,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"On Sat, Feb 7, 2009 at 8:11 AM, Junio C Hamano wrote:\n> Jonathan del Strother writes:\n>\n>> Add a \"Show changes\" option to each prompt in mergetool. This prints the\n>> conflicted changes on the current file, using 'git log -p --merge\n>> '\n>\n> I think the patch should look like this, given the recent conversation I\n> had with you. It seems that the script thinks the unit of indentation is\n> 4-places, and case arms are indented from case/esac (neither of which is\n> the standard git shell script convention), and I tried to match that style\n> used in the existing code.\n>\n> No, I didn't test it.\n>\n> Charles volunteered to take over mergetool, so he is on the Cc: list.\n>\n> git-mergetool.sh | 60 ++++++++++++++++++++++++++++++++++++++++++-----------\n> 1 files changed, 47 insertions(+), 13 deletions(-)\n>\n> diff --git c/git-mergetool.sh w/git-mergetool.sh\n> index 87fa88a..b8604d6 100755\n> --- c/git-mergetool.sh\n> +++ w/git-mergetool.sh\n> @@ -14,6 +14,13 @@ OPTIONS_SPEC=\n> . git-sh-setup\n> require_work_tree\n>\n> +if test -f \"$GIT_DIR/MERGE_HEAD\"\n> +then\n> + in_merge=t show_changes=\", (s)how changes\"\n> +else\n> + in_merge=f show_changes=\n> +fi\n> +\n> # Returns true if the mode reflects a symlink\n> is_symlink () {\n> test \"$1\" = 120000\n> @@ -62,22 +69,28 @@ describe_file () {\n>\n> resolve_symlink_merge () {\n> while true; do\n> - printf \"Use (l)ocal or (r)emote, or (a)bort? \"\n> + printf \"Use (l)ocal or (r)emote$show_changes, or (a)bort? \"\n> read ans\n> - case \"$ans\" in\n> - [lL]*)\n> + case \"$in_merge,$ans\" in\n> + ?,[lL]*)\n> git checkout-index -f --stage=2 -- \"$MERGED\"\n> git add -- \"$MERGED\"\n> cleanup_temp_files --save-backup\n> return 0\n> ;;\n> - [rR]*)\n> + ?,[rR]*)\n> git checkout-index -f --stage=3 -- \"$MERGED\"\n> git add -- \"$MERGED\"\n> cleanup_temp_files --save-backup\n> return 0\n> ;;\n> - [aA]*)\n> + t,[sS]*)\n> + git log -p --merge \"$MERGED\"\n> + printf \"\\n\"\n> + resolve_symlink_merge\n> + return\n> + ;;\n> + ?,[aA]*)\n> return 1\n> ;;\n> esac\n> @@ -87,23 +100,29 @@ resolve_symlink_merge () {\n> resolve_deleted_merge () {\n> while true; do\n> if base_present; then\n> - printf \"Use (m)odified or (d)eleted file, or (a)bort? \"\n> + printf \"Use (m)odified or (d)eleted file$show_changes, or (a)bort? \"\n> else\n> - printf \"Use (c)reated or (d)eleted file, or (a)bort? \"\n> + printf \"Use (c)reated or (d)eleted file$show changes, or (a)bort? \"\n> fi\n> read ans\n> - case \"$ans\" in\n> - [mMcC]*)\n> + case \"$in_merge,$ans\" in\n> + ?,[mMcC]*)\n> git add -- \"$MERGED\"\n> cleanup_temp_files --save-backup\n> return 0\n> ;;\n> - [dD]*)\n> + ?,[dD]*)\n> git rm -- \"$MERGED\" > /dev/null\n> cleanup_temp_files\n> return 0\n> ;;\n> - [aA]*)\n> + t,[sS]*)\n> + git log -p --merge \"$MERGED\"\n> + printf \"\\n\"\n> + resolve_deleted_merge\n> + return\n> + ;;\n> + ?,[aA]*)\n> return 1\n> ;;\n> esac\n> @@ -184,8 +203,23 @@ 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> + case $in_merge in\n> + t) msg_head=\"(S)how changes, or h\" ;;\n> + f) msg_head=\"H\" ;;\n> + esac\n> + print \"${msg_head}it return to start merge resolution tool (%s): \" \"$merge_tool\"\n> + read ans\n> + case \"$in_merge,$ans\" in\n> + t,[sS]*)\n> + git log -p --merge \"$MERGED\"\n> + printf \"\\n\"\n> + ;;\n> + ?,*)\n> + break\n> + ;;\n> + esac\n> + done\n> fi\n>\n> case \"$merge_tool\" in\n>\n\n\nLooks good to me, though there's a minor typo on this line :\nprint \"${msg_head}it return to start merge resolution tool (%s): \" \"$merge_tool\"\n\n- print should be printf\n\nCheers,\nJonathan\n"},{"id":"103677","messageId":"498E3456.1080509@hashpling.org","threadId":"17611","inReplyTo":"57518fd10902070401x14cc7cacrfb8bc88bbf2999cd@mail.gmail.com","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-02-08T01:24:38Z","receivedAt":"2009-02-08T01:24:38Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"Jonathan del Strother wrote:\n> On Sat, Feb 7, 2009 at 8:11 AM, Junio C Hamano wrote:\n>> Jonathan del Strother writes:\n>>\n>>> Add a \"Show changes\" option to each prompt in mergetool. This prints the\n>>> conflicted changes on the current file, using 'git log -p --merge\n>>> '\n>> I think the patch should look like this, given the recent conversation I\n>> had with you. It seems that the script thinks the unit of indentation is\n>> 4-places, and case arms are indented from case/esac (neither of which is\n>> the standard git shell script convention), and I tried to match that style\n>> used in the existing code.\n>>\n>> No, I didn't test it.\n>>\n>> Charles volunteered to take over mergetool, so he is on the Cc: list.\n\nAt the moment, I'm slightly cool towards this patch, but perhaps I don't\n really understand the underlying issue. I understand wanting to check\nsomething (logs) in the middle of a mergetool run but I can't say that\nI've ever wanted to specifically run 'git log -p --merge'. Perhaps some\nusers of mergetool - being visual people - would more naturally reach\nfor gitk?\n\nGiven that mergetool picks up from where it left off when run a second\ntime, what does this patch offer over Ctrl-c, run log tool of your\nchoice, re-run mergetool? Or just running git log in a different\nterminal instance?\n\nI've made couple of minor comments on the patch below.\n\nCharles.\n\n\n>> git-mergetool.sh | 60 ++++++++++++++++++++++++++++++++++++++++++-----------\n>> 1 files changed, 47 insertions(+), 13 deletions(-)\n>>\n>> diff --git c/git-mergetool.sh w/git-mergetool.sh\n>> index 87fa88a..b8604d6 100755\n>> --- c/git-mergetool.sh\n>> +++ w/git-mergetool.sh\n>> @@ -14,6 +14,13 @@ OPTIONS_SPEC=\n>> . git-sh-setup\n>> require_work_tree\n>>\n>> +if test -f \"$GIT_DIR/MERGE_HEAD\"\n>> +then\n>> + in_merge=t show_changes=\", (s)how changes\"\n>> +else\n>> + in_merge=f show_changes=\n>> +fi\n>> +\n>> # Returns true if the mode reflects a symlink\n>> is_symlink () {\n>> test \"$1\" = 120000\n>> @@ -62,22 +69,28 @@ describe_file () {\n>>\n>> resolve_symlink_merge () {\n>> while true; do\n\n--- snip ---\n\n>> ;;\n>> - [aA]*)\n>> + t,[sS]*)\n>> + git log -p --merge \"$MERGED\"\n>> + printf \"\\n\"\n>> + resolve_symlink_merge\n>> + return\n>> + ;;\n\nSlightly unusual recursion, why not just drop out to the 'while true' loop?\n\n>> esac\n>> @@ -87,23 +100,29 @@ resolve_symlink_merge () {\n>> resolve_deleted_merge () {\n>> while true; do\n\n--- snip ---\n\n>> ;;\n>> - [aA]*)\n>> + t,[sS]*)\n>> + git log -p --merge \"$MERGED\"\n>> + printf \"\\n\"\n>> + resolve_deleted_merge\n>> + return\n>> + ;;\n\nResursion as above, but why not fall out to the 'while true' again?\n\n>> @@ -184,8 +203,23 @@ 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>> + case $in_merge in\n>> + t) msg_head=\"(S)how changes, or h\" ;;\n>> + f) msg_head=\"H\" ;;\n>> + esac\n>> + print \"${msg_head}it return to start merge resolution tool (%s): \" \"$merge_tool\"\n>> + read ans\n>> + case \"$in_merge,$ans\" in\n>> + t,[sS]*)\n>> + git log -p --merge \"$MERGED\"\n>> + printf \"\\n\"\n>> + ;;\n\nNo recursion here, this feels a but more natural to me.\n"},{"id":"103732","messageId":"57518fd10902080343p47e30330ufdf2ece909ea0bd9@mail.gmail.com","threadId":"17611","inReplyTo":"498E3456.1080509@hashpling.org","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Jonathan del Strother","fromEmail":"jon.delstrother@bestbefore.tv","sentAt":"2009-02-08T11:43:45Z","receivedAt":"2009-02-08T11:43:45Z","isPatch":true,"sender":{"key":"jon.delstrother@bestbefore.tv","avatar":"https://gravatar.com/avatar/754e21ab701c00e2d21fc261187254c34b2a1c0b959d9ee5be1a295990be3081?d=mp&s=160"},"body":"On 2/8/09, Charles Bailey wrote:\n> Jonathan del Strother wrote:\n>> On Sat, Feb 7, 2009 at 8:11 AM, Junio C Hamano wrote:\n>>> Jonathan del Strother writes:\n>>>\n>>>> Add a \"Show changes\" option to each prompt in mergetool. This prints the\n>>>> conflicted changes on the current file, using 'git log -p --merge\n>>>> '\n>>> I think the patch should look like this, given the recent conversation I\n>>> had with you. It seems that the script thinks the unit of indentation is\n>>> 4-places, and case arms are indented from case/esac (neither of which is\n>>> the standard git shell script convention), and I tried to match that\n>>> style\n>>> used in the existing code.\n>>>\n>>> No, I didn't test it.\n>>>\n>>> Charles volunteered to take over mergetool, so he is on the Cc: list.\n>\n> At the moment, I'm slightly cool towards this patch, but perhaps I don't\n> really understand the underlying issue. I understand wanting to check\n> something (logs) in the middle of a mergetool run but I can't say that\n> I've ever wanted to specifically run 'git log -p --merge'. Perhaps some\n> users of mergetool - being visual people - would more naturally reach\n> for gitk?\n>\n> Given that mergetool picks up from where it left off when run a second\n> time, what does this patch offer over Ctrl-c, run log tool of your\n> choice, re-run mergetool? Or just running git log in a different\n> terminal instance?\n>\n\nA large part of my motivation behind this patch was basically\neducation - my team (and myself) have made poor merge decisions in the\npast, largely due to not being aware of a tool like \"git log --merge\".\nThe patch was attempting to get inexperienced users to make better use\nof such tools. I certainly wouldn't be averse to using gitk instead.\n"},{"id":"103739","messageId":"498ED25F.3020401@hashpling.org","threadId":"17611","inReplyTo":"57518fd10902080343p47e30330ufdf2ece909ea0bd9@mail.gmail.com","subject":"Re: [PATCH] Offer to print changes while running git-mergetool","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-02-08T12:38:55Z","receivedAt":"2009-02-08T12:38:55Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"Jonathan del Strother wrote:\n>> Given that mergetool picks up from where it left off when run a second\n>> time, what does this patch offer over Ctrl-c, run log tool of your\n>> choice, re-run mergetool? Or just running git log in a different\n>> terminal instance?\n>>\n> \n> A large part of my motivation behind this patch was basically\n> education - my team (and myself) have made poor merge decisions in the\n> past, largely due to not being aware of a tool like \"git log --merge\".\n> The patch was attempting to get inexperienced users to make better use\n> of such tools. I certainly wouldn't be averse to using gitk instead.\n\nMy point wasn't that we should use gitk instead, I was just trying to\nillustrate that there may be a whole raft of other commands or\nactivities that a user might want to run to help them with a merge so\nwhy should we hard wire any one of them into mergetool?\n\nThe other doubt that I've since had about this patch is this. Is just\nbefore running the merge tool the best place to offer to show the log?\n\nIn my usual workflows (not necessarily the best!), I usually want to\nfire up the merge tool as quickly as possible to get the merge\nresolutions done.\n\nOnly once I'm in the mergetool do I realise that this one's a bit\ncomplex and I might need to consult the logs to help me resolve this one.\n\nBut now, mergetool is blocked waiting for the merge tool to finish. If I\nabort the merge it's going to offer me the option to abort completely or\ncarry on with the next file. (Perhaps \"try again\" could be a future\ndirection that mergetool might offer, but it doesn't at the moment.)\n\nI'm far more likely to want to consult the logs in a different terminal\n(or gui) with the merge tool still running, especially if I've already\nmerged some of the easy chunks and have only hit difficulties later on\nin the merge.\n\nCharles.\n"}]}