{"thread":{"id":"36155","subject":"[PATCH 1/3] wt-status: Make status messages more consistent with others","startedAt":"2014-03-14T04:37:49Z","lastAt":"2014-03-19T22:30:26Z","messageCount":15,"participants":["Andrew Wong","Marc Branchaud","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"236714","messageId":"1394771872-25940-1-git-send-email-andrew.kw.w@gmail.com","threadId":"36155","inReplyTo":null,"subject":"[PATCH 0/3] Make git more user-friendly during a merge conflict","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-14T04:37:49Z","receivedAt":"2014-03-14T04:37:49Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"2/3: I've added advice.mergeHints to silent the messages that suggests \"git\nmerge--abort\".\n\n3/3: I've added a warning message when users used \"git reset\" during a merge.\nThis warning will be printed if the user is in the middle of a merge. In future\nreleases, we'll change this into an error to prevent work tree from becoming a\nmess.\n\nAndrew Wong (3):\n  wt-status: Make status messages more consistent with others\n  merge: Advise user to use \"git merge --abort\" to abort merges\n  reset: Print a warning when user uses \"git reset\" during a merge\n\n Documentation/config.txt |  3 +++\n advice.c                 |  2 ++\n advice.h                 |  1 +\n builtin/merge.c          |  6 ++++++\n builtin/reset.c          | 21 +++++++++++++++++++++\n wt-status.c              | 23 +++++++++++++----------\n 6 files changed, 46 insertions(+), 10 deletions(-)\n\n-- \n1.9.0.174.g6f75b8f\n"},{"id":"236713","messageId":"1394771872-25940-2-git-send-email-andrew.kw.w@gmail.com","threadId":"36155","inReplyTo":"1394771872-25940-1-git-send-email-andrew.kw.w@gmail.com","subject":"[PATCH 1/3] wt-status: Make status messages more consistent with others","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-14T04:37:50Z","receivedAt":"2014-03-14T04:37:50Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"This is mainly changing messages that say:\n    run \"git foo --bar\"\nto\n    use \"git foo --bar\" to baz\n\nAlthough the commands and flags are usually self-explanatory, this is\nmore consistent with other status messages, and gives some sort of\nexplanation to the user.\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n wt-status.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex a452407..9f2358a 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -899,13 +899,13 @@ static void show_merge_in_progress(struct wt_status *s,\n \t\tstatus_printf_ln(s, color, _(\"You have unmerged paths.\"));\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (fix conflicts and run \\\"git commit\\\")\"));\n+\t\t\t\t_(\"  (fix conflicts and use \\\"git commit\\\" to conclude the merge)\"));\n \t} else {\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (use \\\"git commit\\\" to conclude merge)\"));\n+\t\t\t\t_(\"  (use \\\"git commit\\\" to conclude the merge)\"));\n \t}\n \twt_status_print_trailer(s);\n }\n@@ -922,7 +922,7 @@ static void show_am_in_progress(struct wt_status *s,\n \tif (s->hints) {\n \t\tif (!state->am_empty_patch)\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (fix conflicts and then run \\\"git am --continue\\\")\"));\n+\t\t\t\t_(\"  (fix conflicts and then use \\\"git am --continue\\\" to continue)\"));\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"  (use \\\"git am --skip\\\" to skip this patch)\"));\n \t\tstatus_printf_ln(s, color,\n@@ -994,7 +994,7 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\"You are currently rebasing.\"));\n \t\tif (s->hints) {\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (fix conflicts and then run \\\"git rebase --continue\\\")\"));\n+\t\t\t\t_(\"  (fix conflicts and then use \\\"git rebase --continue\\\" to continue)\"));\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\"  (use \\\"git rebase --skip\\\" to skip this patch)\"));\n \t\t\tstatus_printf_ln(s, color,\n@@ -1011,7 +1011,7 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\"You are currently rebasing.\"));\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (all conflicts fixed: run \\\"git rebase --continue\\\")\"));\n+\t\t\t\t_(\"  (all conflicts fixed: use \\\"git rebase --continue\\\" to continue)\"));\n \t} else if (split_commit_in_progress(s)) {\n \t\tif (state->branch)\n \t\t\tstatus_printf_ln(s, color,\n@@ -1023,7 +1023,7 @@ static void show_rebase_in_progress(struct wt_status *s,\n \t\t\t\t\t _(\"You are currently splitting a commit during a rebase.\"));\n \t\tif (s->hints)\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (Once your working directory is clean, run \\\"git rebase --continue\\\")\"));\n+\t\t\t\t_(\"  (Once your working directory is clean, use \\\"git rebase --continue\\\" to continue)\"));\n \t} else {\n \t\tif (state->branch)\n \t\t\tstatus_printf_ln(s, color,\n@@ -1052,10 +1052,10 @@ static void show_cherry_pick_in_progress(struct wt_status *s,\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (fix conflicts and run \\\"git cherry-pick --continue\\\")\"));\n+\t\t\t\t_(\"  (fix conflicts and use \\\"git cherry-pick --continue\\\" to continue)\"));\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (all conflicts fixed: run \\\"git cherry-pick --continue\\\")\"));\n+\t\t\t\t_(\"  (all conflicts fixed: use \\\"git cherry-pick --continue\\\" to continue)\"));\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"  (use \\\"git cherry-pick --abort\\\" to cancel the cherry-pick operation)\"));\n \t}\n@@ -1071,10 +1071,10 @@ static void show_revert_in_progress(struct wt_status *s,\n \tif (s->hints) {\n \t\tif (has_unmerged(s))\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (fix conflicts and run \\\"git revert --continue\\\")\"));\n+\t\t\t\t_(\"  (fix conflicts and use \\\"git revert --continue\\\" to continue)\"));\n \t\telse\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\t_(\"  (all conflicts fixed: run \\\"git revert --continue\\\")\"));\n+\t\t\t\t_(\"  (all conflicts fixed: use \\\"git revert --continue\\\" to continue)\"));\n \t\tstatus_printf_ln(s, color,\n \t\t\t_(\"  (use \\\"git revert --abort\\\" to cancel the revert operation)\"));\n \t}\n-- \n1.9.0.174.g6f75b8f\n"},{"id":"236716","messageId":"1394771872-25940-3-git-send-email-andrew.kw.w@gmail.com","threadId":"36155","inReplyTo":"1394771872-25940-1-git-send-email-andrew.kw.w@gmail.com","subject":"[PATCH 2/3] merge: Advise user to use \"git merge --abort\" to abort merges","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-14T04:37:51Z","receivedAt":"2014-03-14T04:37:51Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"Print message during \"git merge\" and \"git status\".\n\nAdd a new \"mergeHints\" advice to silence these messages.\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n Documentation/config.txt | 3 +++\n advice.c                 | 2 ++\n advice.h                 | 1 +\n builtin/merge.c          | 6 ++++++\n wt-status.c              | 3 +++\n 5 files changed, 15 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 73904bc..936a20b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -196,6 +196,9 @@ advice.*::\n \trmHints::\n \t\tIn case of failure in the output of linkgit:git-rm[1],\n \t\tshow directions on how to proceed from the current state.\n+\tmergeHints::\n+                Show directions on how to proceed from the current state in the\n+                output of linkgit:git-merge[1].\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex 486f823..e910734 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -15,6 +15,7 @@ int advice_detached_head = 1;\n int advice_set_upstream_failure = 1;\n int advice_object_name_warning = 1;\n int advice_rm_hints = 1;\n+int advice_merge_hints = 1;\n \n static struct {\n \tconst char *name;\n@@ -35,6 +36,7 @@ static struct {\n \t{ \"setupstreamfailure\", &advice_set_upstream_failure },\n \t{ \"objectnamewarning\", &advice_object_name_warning },\n \t{ \"rmhints\", &advice_rm_hints },\n+\t{ \"mergehints\", &advice_merge_hints },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushnonfastforward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex 5ecc6c1..d337f1c 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -18,6 +18,7 @@ extern int advice_detached_head;\n extern int advice_set_upstream_failure;\n extern int advice_object_name_warning;\n extern int advice_rm_hints;\n+extern int advice_merge_hints;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex f0cf120..c55ac03 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -805,6 +805,8 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)\n \t\terror(\"%s\", err_msg);\n \tfprintf(stderr,\n \t\t_(\"Not committing merge; use 'git commit' to complete the merge.\\n\"));\n+\tif (advice_merge_hints)\n+\t\tprintf(_(\"  (use \\\"git merge --abort\\\" to abort the merge)\\n\"));\n \twrite_merge_state(remoteheads);\n \texit(1);\n }\n@@ -913,6 +915,8 @@ static int suggest_conflicts(int renormalizing)\n \trerere(allow_rerere_auto);\n \tprintf(_(\"Automatic merge failed; \"\n \t\t\t\"fix conflicts and then commit the result.\\n\"));\n+\tif (advice_merge_hints)\n+\t\tprintf(_(\"  (use \\\"git merge --abort\\\" to abort the merge)\\n\"));\n \treturn 1;\n }\n \n@@ -1559,6 +1563,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tif (merge_was_ok)\n \t\tfprintf(stderr, _(\"Automatic merge went well; \"\n \t\t\t\"stopped before committing as requested\\n\"));\n+\tif (advice_merge_hints)\n+\t\tprintf(_(\"  (use \\\"git merge --abort\\\" to abort the merge)\\n\"));\n \telse\n \t\tret = suggest_conflicts(option_renormalize);\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 9f2358a..3b30bf9 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -907,6 +907,9 @@ static void show_merge_in_progress(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color,\n \t\t\t\t_(\"  (use \\\"git commit\\\" to conclude the merge)\"));\n \t}\n+\tif (s->hints)\n+\t\tstatus_printf_ln(s, color,\n+\t\t\t_(\"  (use \\\"git merge --abort\\\" to abort the merge)\"));\n \twt_status_print_trailer(s);\n }\n \n-- \n1.9.0.174.g6f75b8f\n"},{"id":"236715","messageId":"1394771872-25940-4-git-send-email-andrew.kw.w@gmail.com","threadId":"36155","inReplyTo":"1394771872-25940-1-git-send-email-andrew.kw.w@gmail.com","subject":"[PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-14T04:37:52Z","receivedAt":"2014-03-14T04:37:52Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"During a merge, \"--mixed\" is most likely not what the user wants. Using\n\"--mixed\" during a merge would leave the merged changes and new files\nmixed in with the local changes. The user would have to manually clean\nup the work tree, which is non-trivial. In future releases, we want to\nmake \"git reset\" error out when used in the middle of a merge. For now,\nwe simply print out a warning to the user.\n\nSigned-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n---\n builtin/reset.c | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4fd1c6c..04e8103 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -331,8 +331,29 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t_(reset_type_names[reset_type]));\n \t}\n \tif (reset_type == NONE)\n+\t{\n \t\treset_type = MIXED; /* by default */\n \n+\t\t/* During a merge, \"--mixed\" is most likely not what the user\n+\t\t * wants. Using \"--mixed\" during a merge would leave the merged\n+\t\t * changes and new files mixed in with the local changes. The\n+\t\t * user would have to manually clean up the work tree, which is\n+\t\t * non-trivial. In future releases, we want to make \"git reset\"\n+\t\t * error out when used in the middle of a merge. For now, we\n+\t\t * simply print out a warning to the user. */\n+\t\tif (is_merge())\n+\t\t\twarning(_(\"You have used 'git reset' in the middle of a merge. 'git reset' defaults to\\n\"\n+\t\t\t\t  \"'git reset --mixed', which means git will not clean up any merged changes and\\n\"\n+\t\t\t\t  \"new files that were created in the work tree. It also becomes impossible for\\n\"\n+\t\t\t\t  \"git to automatically clean up the work tree later, so you would have to clean\\n\"\n+\t\t\t\t  \"up the work tree manually. To avoid this next time, you may want to use 'git\\n\"\n+\t\t\t\t  \"reset --merge', or equivalently 'git merge --abort'.\\n\"\n+\t\t\t\t  \"\\n\"\n+\t\t\t\t  \"In future releases, using 'git reset' in the middle of a merge will result in\\n\"\n+\t\t\t\t  \"an error.\"\n+\t\t\t\t ));\n+\t}\n+\n \tif (reset_type != SOFT && reset_type != MIXED)\n \t\tsetup_work_tree();\n \n-- \n1.9.0.174.g6f75b8f\n"},{"id":"236726","messageId":"5323131C.7070506@xiplink.com","threadId":"36155","inReplyTo":"1394771872-25940-4-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2014-03-14T14:33:00Z","receivedAt":"2014-03-14T14:33:00Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 14-03-14 12:37 AM, Andrew Wong wrote:\n> During a merge, \"--mixed\" is most likely not what the user wants. Using\n> \"--mixed\" during a merge would leave the merged changes and new files\n> mixed in with the local changes. The user would have to manually clean\n> up the work tree, which is non-trivial. In future releases, we want to\n> make \"git reset\" error out when used in the middle of a merge. For now,\n> we simply print out a warning to the user.\n\nI know this approach was suggested earlier, but given these dangers it seems\nsilly to give this big warning on a plain \"git reset\" but still go ahead and\ndo the things the warning talks about.\n\nIs there any issue with changing \"git reset\" to error-out now but letting\n\"git reset --mixed\" proceed?  Something like (note the reworded warning message):\n\n$ git reset\nCowardly refusing to implicitly run 'git reset --mixed' during a merge.\nThis would not clean up any merged changes and would not remove any new\nfiles that were created in the work tree.  It would also make it impossible\nfor git to automatically clean up the work tree later, so you would have to\nclean up the work tree manually.\nYou probably meant to run 'git merge --abort' instead.\n$ git reset --mixed   # Stoopid git!  I know what I'm doing!\n$\n\nThis would mean that the 10% of git users who like to do \"git reset\" in the\nmiddle of a conflicted merge will have to teach their fingers some extra\nmotions.  But these users are all veterans, and they can more easily type in\n8 extra characters (6 with completion) than new users can recover from\naccidentally misusing git-reset's power.\n\n\t\tM.\n"},{"id":"236734","messageId":"CADgNjan9kCTMPczFzO4jQvM63EU4x7KnJKszhno5PjHivE9ENg@mail.gmail.com","threadId":"36155","inReplyTo":"5323131C.7070506@xiplink.com","subject":"Re: [PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-14T17:04:42Z","receivedAt":"2014-03-14T17:04:42Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:\n> I know this approach was suggested earlier, but given these dangers it seems\n> silly to give this big warning on a plain \"git reset\" but still go ahead and\n> do the things the warning talks about.\n>\n> Is there any issue with changing \"git reset\" to error-out now but letting\n> \"git reset --mixed\" proceed?  Something like (note the reworded warning message):\n\nYeah, I would have preferred to have \"git reset\" error out right now,\nbecause the messed up work tree can be quite a pain to clean up. The\nmain argument for issuing the warning is about maintaining\ncompatibility.\n\nFor the users that really did mean \"--merge\", the warning is silly.\nIt's basically saying \"We know that you're about to mess up your work\ntree, but we let you mess up anyway. Learn the correct way so that you\ndon't mess up next time\".\n\nIt actually doesn't seem too bad if we did make \"git reset\" to error\nout (during a merge) right away. By erroring out, the command won't\ncause some irreversible damage, and users don't lose data. Yes, it\nbreaks compatibility, but perhaps not in a bad way?\n\nI'm really fine with either. Junio?\n"},{"id":"236745","messageId":"xmqqa9csh54f.fsf@gitster.dls.corp.google.com","threadId":"36155","inReplyTo":"CADgNjan9kCTMPczFzO4jQvM63EU4x7KnJKszhno5PjHivE9ENg@mail.gmail.com","subject":"Re: [PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-14T20:55:44Z","receivedAt":"2014-03-14T20:55:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:\n>> I know this approach was suggested earlier, but given these dangers it seems\n>> silly to give this big warning on a plain \"git reset\" but still go ahead and\n>> do the things the warning talks about.\n>>\n>> Is there any issue with changing \"git reset\" to error-out now but letting\n>> \"git reset --mixed\" proceed?  Something like (note the reworded warning message):\n>\n> Yeah, I would have preferred to have \"git reset\" error out right now,\n> because the messed up work tree can be quite a pain to clean up. The\n> main argument for issuing the warning is about maintaining\n> compatibility.\n>\n> For the users that really did mean \"--merge\", the warning is silly.\n> It's basically saying \"We know that you're about to mess up your work\n> tree, but we let you mess up anyway. Learn the correct way so that you\n> don't mess up next time\".\n\nI suspect that you meant \"--mixed\" instead of \"--merge\" here.\n\nI do not agree with the \"We know that you are about to mess up\"\nabove.  Whoever is issuing that warning message may not know better\nthan the user who is running \"reset\".  As you wrote \"most likely not\nwhat the user wants\" in your proposed log message, the only thing we\nknow is that it _often_ is a newbie mistake.\n\nI recently needed to manually cherry-pick only one half of a patch,\nand the way I did so was\n\n\tgit show $that_commit >P.diff\n        git apply -3 P.diff\n        ... conflicts are expected; that is why I used -3 in the\n        ... first place\n        git reset\n        git diff HEAD\n\tedit\n        ... edit away the other half I did not want to cherry-pick\n        ... while fixing the conflicted parts that happened to be\n        ... in the part I did want to cherry-pick\n\n\"git cherry-pick --no-commit $that_commit\" could have been used as\na replacement for the first two steps but being able to run the\n\"reset\" without erroring out was an essential part to make this\nworkflow usable.\n\nSo I am OK with \"eventually error out by default\", but not OK with\n\"we know better than the user and will not allow it at all\".\n"},{"id":"236751","messageId":"CADgNjakXANkNPyO6n5CH3=R-Heuf2qQ0STyJU4zSgTw63C19-A@mail.gmail.com","threadId":"36155","inReplyTo":"xmqqa9csh54f.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-14T21:35:19Z","receivedAt":"2014-03-14T21:35:19Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"On Fri, Mar 14, 2014 at 4:55 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> For the users that really did mean \"--merge\", the warning is silly.\n>> It's basically saying \"We know that you're about to mess up your work\n>> tree, but we let you mess up anyway. Learn the correct way so that you\n>> don't mess up next time\".\n>\n> I suspect that you meant \"--mixed\" instead of \"--merge\" here.\n\nNo, I did mean \"--merge\". It's silly for inexperienced users because\nit's too late to use \"--merge\" by the time they realized they should\nnot have used the default. The work tree has already become a mess. So\nthey'd immediately think \"if git was smart enough to warn me about the\nmess, why not prevent me from getting into the mess in the first\nplace?\"\n\nFor the experienced users, they would understand the warning, because\nthey would be aware of the index, and the effect that \"--mixed\" and\n\"--merge\" have on it.\n\n> So I am OK with \"eventually error out by default\", but not OK with\n> \"we know better than the user and will not allow it at all\".\n\nAgain, I didn't mean \"we know better than the user\". However, from a\nnew user's perspective, they won't understand why \"git reset\" gives\nthe warning, but still \"knowingly\" messes up their work tree.\n\nAnd \"we don't know better than the user\" is exactly why I think we\nshould \"eventually error out\" rather than automatically switching to\n\"--merge\". As Matthieu was saying, automatically switching to\n\"--merge\" could discard conflict resolutions, which would be\nundesirable. So it's better for git to error out then having git\ndecides what the user (probably) wants.\n"},{"id":"236801","messageId":"5324A8BF.80308@xiplink.com","threadId":"36155","inReplyTo":"xmqqa9csh54f.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2014-03-15T19:23:43Z","receivedAt":"2014-03-15T19:23:43Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 14-03-14 04:55 PM, Junio C Hamano wrote:\n>\n> So I am OK with \"eventually error out by default\", but not OK with\n> \"we know better than the user and will not allow it at all\".\n\nCan I interpret that as you being OK with my proposed \"Cowardly \nrefusing\" approach?\n\n\t\tM.\n"},{"id":"236921","messageId":"xmqq1ty0cx3l.fsf@gitster.dls.corp.google.com","threadId":"36155","inReplyTo":"1394771872-25940-2-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH 1/3] wt-status: Make status messages more consistent with others","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T21:51:42Z","receivedAt":"2014-03-17T21:51:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> This is mainly changing messages that say:\n>     run \"git foo --bar\"\n> to\n>     use \"git foo --bar\" to baz\n\n\"git foo --bar\" is fine, but \"to baz\" was hard to read without first\nrealizing that 'baz' stands for some/any verb.  I think rephrasing\nit to\n\n\tuse \"git foo --bar\" to do baz\n\nwould reduce confusion.\n\n> diff --git a/wt-status.c b/wt-status.c\n> index a452407..9f2358a 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -899,13 +899,13 @@ static void show_merge_in_progress(struct wt_status *s,\n>  \t\tstatus_printf_ln(s, color, _(\"You have unmerged paths.\"));\n>  \t\tif (s->hints)\n>  \t\t\tstatus_printf_ln(s, color,\n> -\t\t\t\t_(\"  (fix conflicts and run \\\"git commit\\\")\"));\n> +\t\t\t\t_(\"  (fix conflicts and use \\\"git commit\\\" to conclude the merge)\"));\n>  \t} else {\n>  \t\tstatus_printf_ln(s, color,\n>  \t\t\t_(\"All conflicts fixed but you are still merging.\"));\n>  \t\tif (s->hints)\n>  \t\t\tstatus_printf_ln(s, color,\n> -\t\t\t\t_(\"  (use \\\"git commit\\\" to conclude merge)\"));\n> +\t\t\t\t_(\"  (use \\\"git commit\\\" to conclude the merge)\"));\n>  \t}\n>  \twt_status_print_trailer(s);\n>  }\n\nThe above hunk makes sense.\n\nAt first glance, I felt that none of the remainder made much sense.\nMy reaction was: \"git foo --continue\" to continue?  What else could\nthe --continue option even mean?\n\nThe real value I see in these conversions is by saying \"use this to\ncontinue\" instead of an unconditional \"run this\", it implies \"*IF*\nyou wanted to continue, you can do this\", meaning that user also has\nthe option of *not* continuing.  But the proposed update falls short\nof realizing the full potential, if that is the value we are trying\nto add.  I'd say\n\n\tfix conflicts and then use \"git am --continue\" if you want\n\tto continue.\n\nor an even more explicit\n\n\tfix conflicts and then use \"git am --continue\" if you want\n\tto continue; or you can \"git am --abort\" to discontinue.\n\nwould be an improvement, but\n\n\tfix conflicts and then use \"git am --continue\" to continue\n\nis probably not quite.\n\n> @@ -922,7 +922,7 @@ static void show_am_in_progress(struct wt_status *s,\n>  \tif (s->hints) {\n>  \t\tif (!state->am_empty_patch)\n>  \t\t\tstatus_printf_ln(s, color,\n> -\t\t\t\t_(\"  (fix conflicts and then run \\\"git am --continue\\\")\"));\n> +\t\t\t\t_(\"  (fix conflicts and then use \\\"git am --continue\\\" to continue)\"));\n>  \t\tstatus_printf_ln(s, color,\n>  \t\t\t_(\"  (use \\\"git am --skip\\\" to skip this patch)\"));\n>  \t\tstatus_printf_ln(s, color,\n"},{"id":"236924","messageId":"xmqqwqfsbif9.fsf@gitster.dls.corp.google.com","threadId":"36155","inReplyTo":"1394771872-25940-4-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH 3/3] reset: Print a warning when user uses \"git reset\" during a merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T21:54:02Z","receivedAt":"2014-03-17T21:54:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> During a merge, \"--mixed\" is most likely not what the user wants. Using\n> \"--mixed\" during a merge would leave the merged changes and new files\n> mixed in with the local changes. The user would have to manually clean\n> up the work tree, which is non-trivial. In future releases, we want to\n> make \"git reset\" error out when used in the middle of a merge. For now,\n> we simply print out a warning to the user.\n>\n> Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>\n> ---\n>  builtin/reset.c | 21 +++++++++++++++++++++\n>  1 file changed, 21 insertions(+)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 4fd1c6c..04e8103 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -331,8 +331,29 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>  \t\t\t\t\t_(reset_type_names[reset_type]));\n>  \t}\n>  \tif (reset_type == NONE)\n> +\t{\n>  \t\treset_type = MIXED; /* by default */\n>  \n> +\t\t/* During a merge, \"--mixed\" is most likely not what the user\n\nTwo style niggles here.\n\n> +\t\t * wants. Using \"--mixed\" during a merge would leave the merged\n> +\t\t * changes and new files mixed in with the local changes. The\n> +\t\t * user would have to manually clean up the work tree, which is\n> +\t\t * non-trivial. In future releases, we want to make \"git reset\"\n\n\"we want\"?  Has any of us decided on that?\n\n> +\t\t * error out when used in the middle of a merge. For now, we\n> +\t\t * simply print out a warning to the user. */\n> +\t\tif (is_merge())\n> +\t\t\twarning(_(\"You have used 'git reset' in the middle of a merge. 'git reset' defaults to\\n\"\n> +\t\t\t\t  \"'git reset --mixed', which means git will not clean up any merged changes and\\n\"\n> +\t\t\t\t  \"new files that were created in the work tree. It also becomes impossible for\\n\"\n> +\t\t\t\t  \"git to automatically clean up the work tree later, so you would have to clean\\n\"\n> +\t\t\t\t  \"up the work tree manually. To avoid this next time, you may want to use 'git\\n\"\n> +\t\t\t\t  \"reset --merge', or equivalently 'git merge --abort'.\\n\"\n> +\t\t\t\t  \"\\n\"\n> +\t\t\t\t  \"In future releases, using 'git reset' in the middle of a merge will result in\\n\"\n> +\t\t\t\t  \"an error.\"\n> +\t\t\t\t ));\n> +\t}\n> +\n>  \tif (reset_type != SOFT && reset_type != MIXED)\n>  \t\tsetup_work_tree();\n"},{"id":"236926","messageId":"xmqqr460bi7i.fsf@gitster.dls.corp.google.com","threadId":"36155","inReplyTo":"1394771872-25940-3-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH 2/3] merge: Advise user to use \"git merge --abort\" to abort merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T21:58:41Z","receivedAt":"2014-03-17T21:58:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> Print message during \"git merge\" and \"git status\".\n>\n> Add a new \"mergeHints\" advice to silence these messages.\n\nThis sounds sensible.  Don't we want to have this one take effect on\nthe places where advice.resolveConflict is used in git-pull?\nI.e. something like:\n\n\tdo_we_advise=no\n\tif advice.resolveConflict is not set:\n\t\tif advice.mergeHints is set to false:\n                \tdo_we_advise=no\n\t\telse:\n                \tdo_we_advise=yes\n\telse:\n        \tdo_we_advise=yes\n\n        if do_we_advise == 'yes':\n        \tgive advice in die_conflict and die_merge\n"},{"id":"236938","messageId":"xmqqha6wa0ln.fsf@gitster.dls.corp.google.com","threadId":"36155","inReplyTo":"1394771872-25940-1-git-send-email-andrew.kw.w@gmail.com","subject":"Re: [PATCH 0/3] Make git more user-friendly during a merge conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T23:04:20Z","receivedAt":"2014-03-17T23:04:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> 2/3: I've added advice.mergeHints to silent the messages that suggests \"git\n> merge--abort\".\n>\n> 3/3: I've added a warning message when users used \"git reset\" during a merge.\n> This warning will be printed if the user is in the middle of a merge. In future\n> releases, we'll change this into an error to prevent work tree from becoming a\n> mess.\n>\n> Andrew Wong (3):\n>   wt-status: Make status messages more consistent with others\n>   merge: Advise user to use \"git merge --abort\" to abort merges\n>   reset: Print a warning when user uses \"git reset\" during a merge\n>\n>  Documentation/config.txt |  3 +++\n>  advice.c                 |  2 ++\n>  advice.h                 |  1 +\n>  builtin/merge.c          |  6 ++++++\n>  builtin/reset.c          | 21 +++++++++++++++++++++\n>  wt-status.c              | 23 +++++++++++++----------\n>  6 files changed, 46 insertions(+), 10 deletions(-)\n\nHas this series been tested with existing test suite?  I tentatively\nqueued it to 'pu' but then had to revert because many tests started\nfailing, causing me to redo the today's integration cycle for 'pu'\nonce again.\n"},{"id":"236942","messageId":"CADgNjakRSw-S4VbKnLC9PpmAcEi7iO=r0SBEy2XO3XhtDq=uJg@mail.gmail.com","threadId":"36155","inReplyTo":"xmqqha6wa0ln.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] Make git more user-friendly during a merge conflict","fromName":"Andrew Wong","fromEmail":"andrew.kw.w@gmail.com","sentAt":"2014-03-17T23:25:00Z","receivedAt":"2014-03-17T23:25:00Z","isPatch":true,"sender":{"key":"andrew.kw.w@gmail.com","avatar":"https://avatars.githubusercontent.com/u/489311?v=4"},"body":"On Mon, Mar 17, 2014 at 7:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Has this series been tested with existing test suite?  I tentatively\n> queued it to 'pu' but then had to revert because many tests started\n> failing, causing me to redo the today's integration cycle for 'pu'\n> once again.\n\nI tested it during RFC, but missed it when I sent it as patch. The\nproblem is here:\n\n@@ -1559,6 +1563,8 @@ int cmd_merge(int argc, const char **argv, const\nchar *prefix)\n        if (merge_was_ok)\n                fprintf(stderr, _(\"Automatic merge went well; \"\n                        \"stopped before committing as requested\\n\"));\n+       if (advice_merge_hints)\n+               printf(_(\"  (use \\\"git merge --abort\\\" to abort the merge)\\n\"));\n        else\n                ret = suggest_conflicts(option_\nrenormalize);\n\nI'll fix the problem. Sorry about that.\n"},{"id":"237127","messageId":"xmqqd2hh254t.fsf@gitster.dls.corp.google.com","threadId":"36155","inReplyTo":"CADgNjakRSw-S4VbKnLC9PpmAcEi7iO=r0SBEy2XO3XhtDq=uJg@mail.gmail.com","subject":"Re: [PATCH 0/3] Make git more user-friendly during a merge conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T22:30:26Z","receivedAt":"2014-03-19T22:30:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wong <andrew.kw.w@gmail.com> writes:\n\n> On Mon, Mar 17, 2014 at 7:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Has this series been tested with existing test suite? ...\n> I tested it during RFC, but missed it when I sent it as patch.\n> ...\n> I'll fix the problem. Sorry about that.\n\nThanks.  Will hold onto the topic branch lest I forget, but will\nkeep it out of 'pu' in the meantime.\n"}]}