{"thread":{"id":"22103","subject":"[RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","startedAt":"2010-01-06T13:10:14Z","lastAt":"2010-01-13T06:56:58Z","messageCount":14,"participants":["Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"130919","messageId":"1262783414-23101-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22103","inReplyTo":null,"subject":"[RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-06T13:10:14Z","receivedAt":"2010-01-06T13:10:14Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The error message used to look like this:\n\n$ git commit\nfoo.txt: needs merge\nfoo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\nfoo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\nfoo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\nerror: Error building trees\n\nThe \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\noption to make the output more consistant with the other porcelain\ncommands, and catch the error in return, to stop with a clean error\nmessage. The next lines were displayed by a call to cache_tree_update(),\nwhich is not reached anymore if we noticed the conflict.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nAs usual, I try to have error messages point to the solution, not just\nthe origin of the problem.\n\nTwo questions:\n\n* Did anyone actually use the 3 \"file: unmerged (sha1)\" lines?\n\n* Do you like my new message?\n\nThanks,\n\n builtin-commit.c |   16 ++++++++++++++--\n 1 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 3dfcd77..d491a01 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -235,6 +235,18 @@ static void create_base_index(void)\n \t\texit(128); /* We've already reported the error, finish dying */\n }\n \n+static void refresh_cache_or_die(int refresh_flags)\n+{\n+\t/*\n+\t * refresh_flags contains REFRESH_QUIET, so the only errors\n+\t * are for unmerged entries.\n+\t*/\n+\tif(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))\n+\t\tdie(\"Commit is not possible because you have unmerged files.\\n\"\n+\t\t    \"Please, resolve the conflicts listed above, and then mark them\\n\"\n+\t\t    \"as resolved with 'git add <filename>', or use 'git commit -a'.\");\n+}\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)\n {\n \tint fd;\n@@ -274,7 +286,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (all || (also && pathspec && *pathspec)) {\n \t\tint fd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n@@ -293,7 +305,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n-- \n1.6.6.76.gd6b23.dirty\n"},{"id":"130928","messageId":"7veim3yx5o.fsf@alter.siamese.dyndns.org","threadId":"22103","inReplyTo":"1262783414-23101-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-06T17:13:07Z","receivedAt":"2010-01-06T17:13:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> The error message used to look like this:\n>\n> $ git commit\n> foo.txt: needs merge\n> foo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\n> foo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\n> foo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\n> error: Error building trees\n>\n> The \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\n> option to make the output more consistant with the other porcelain\n> commands, and catch the error in return, to stop with a clean error\n> message. The next lines were displayed by a call to cache_tree_update(),\n> which is not reached anymore if we noticed the conflict.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> As usual, I try to have error messages point to the solution, not just\n> the origin of the problem.\n\nSeems to be a sensible approach to me.  \n\n> Two questions:\n>\n> * Did anyone actually use the 3 \"file: unmerged (sha1)\" lines?\n\nI don't think anybody does these days, and even if the question were about\nthe historical practice, I doubt anybody used \"git merge-file\" using these\nblob object names to come up with a resolution.  Besides, if they really\nwant to, they can ask \"ls-files -u\" for that information themselves. \n\n> * Do you like my new message?\n\nThat would have been much easier to answer if you wrote \"Here is an error\nmessage produced with this patch\" in the proposed commit log message.  I\nhaven't applied nor read your patch carefully, but I imagine you would say\nsomething like this?\n\n  With this patch, the error looks like this:\n\n  $ git commit\n  U foo.txt\n  fatal: Commit is not possible because you have unmerged files.\n  Please, resolve the conflicts listed above, and then mark them\n  as resolved with 'git add <filename>', or use 'git commit -a'\n\nDo I like it?  Hmmm.\n\n - \"Please, \", especially with the comma, looks superfluous;\n\n - Some people consider \"please resolve the conflicts\" to mean the whole\n   process of \"fix them up in the work tree, and mark them resolved in the\n   index\", and others consider it to mean only the first step to \"fix them\n   up in the work tree\".  I happen to be in the former camp, and to me\n   \",and then mark them as..., add <filename>',\" look superfluous because\n   of that.\n\n   I however think it is more helpful to new people who benefit from this\n   advice message if we spell out these two steps.  Unfortunately, for\n   that purpose, the description of the latter \"mark them resolved in the\n   index\" step you have looks inadequate; e.g. sometimes it needs to be\n   done with \"git rm <filename>\".\n\nThe need to give this advice on how to resolve conflicts is shared among\ncommands like 'git merge', 'git cherry-pick', and 'git status', to name a\nfew.  I think we should consolidate them all.\n\n - Decide if we go one-step (i.e. just say \"resolve conflicts\" and nothing\n   else) or two-step (i.e. say \"Fix them up in the work tree\" and \"mark\n   them resolved in the index\") approach; I am leaning toward the latter.\n\n - Decide the exact language used.  I think \"Fix them up in the work tree\"\n   is passably okay, but \"Mark them resolved in the index\" needs to be\n   made more concrete, and \"git add <filename>\" is not quite it, I am\n   afraid.\n\n - Omit issuing the advisory message when advice.resolveConflict is set to\n   false.\n\n - Perhaps have a common helper function to do this from the commands that\n   need to give the advice.\n\nThanks.\n"},{"id":"130931","messageId":"vpqtyuz86ny.fsf@bauges.imag.fr","threadId":"22103","inReplyTo":"7veim3yx5o.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-01-06T17:49:53Z","receivedAt":"2010-01-06T17:49:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> Two questions:\n>>\n>> * Did anyone actually use the 3 \"file: unmerged (sha1)\" lines?\n>\n> I don't think anybody does these days, and even if the question were about\n> the historical practice, I doubt anybody used \"git merge-file\" using these\n> blob object names to come up with a resolution.  Besides, if they really\n> want to, they can ask \"ls-files -u\" for that information themselves. \n\nOK, that's what I thought, but 'wanted to be sure.\n\n>I imagine you would say\n> something like this?\n>\n>   With this patch, the error looks like this:\n>\n>   $ git commit\n>   U foo.txt\n>   fatal: Commit is not possible because you have unmerged files.\n>   Please, resolve the conflicts listed above, and then mark them\n>   as resolved with 'git add <filename>', or use 'git commit -a'\n\nYes, sorry for not making it more explicit.\n\n> Do I like it?  Hmmm.\n>\n>  - \"Please, \", especially with the comma, looks superfluous;\n\nI wouldn't call it mandatory, but it doesn't harm, and\n\n$ grep -i -n -e '\"Please' *.c\nbuiltin-commit.c:246:\t\t    \"Please, resolve the conflicts listed above, and then mark them\\n\"\nbuiltin-commit.c:698:\t\t\t\"Please supply the message using either -m or -F option.\\n\");\nbuiltin-help.c:209:\t\t\t\"Please consider using 'man.<tool>.cmd' instead.\",\nbuiltin-help.c:221:\t\t\t\"Please consider using 'man.<tool>.path' instead.\",\nbuiltin-remote.c:1124:\t\t\t\t\t\"Please choose one explicitly with:\");\nbuiltin-tag.c:321:\t\t\t\"Please supply the message using either -m or -F option.\\n\");\nmerge-recursive.c:1191:\t\t\t\"Please, commit your changes or stash them before you can merge.\";\nsetup.c:241:\t\twarning(\"Please upgrade Git\");\n\n>  - Some people consider \"please resolve the conflicts\" to mean the whole\n>    process of \"fix them up in the work tree, and mark them resolved in the\n>    index\", and others consider it to mean only the first step to \"fix them\n>    up in the work tree\".  I happen to be in the former camp, and to me\n>    \",and then mark them as..., add <filename>',\" look superfluous because\n>    of that.\n>\n>    I however think it is more helpful to new people who benefit from this\n>    advice message if we spell out these two steps.\n\nYes. Someone having \"resolved\" (i.e. removed markers), and trying to\ncommit should get a hint on the commands he can use to achieve this\ngoal.\n\n>    Unfortunately, for that purpose, the description of the latter\n>    \"mark them resolved in the index\" step you have looks inadequate;\n>    e.g. sometimes it needs to be done with \"git rm <filename>\".\n\nMaybe \"sometimes\", but to me, that's sufficiently rare to be omited\nhere (I don't think I ever used 'git rm' to resolve a conflict). The\nuser manual says this:\n\n,----\n| Each time you resolve the conflicts in a file and update the index:\n| \n| $ git add file.txt\n| \n| the different stages of that file will be \"collapsed\", after which git\n| diff will (by default) no longer show diffs for that file.\n`----\n\nand I don't think it makes sense to try to be more exhaustive here\nthan in the user-manual.\n\n> The need to give this advice on how to resolve conflicts is shared among\n> commands like 'git merge', 'git cherry-pick', and 'git status', to name a\n> few.\n\nNot sure 'status' needs anything more. It already says\n\n# Unmerged paths:\n#   (use \"git add/rm <file>...\" as appropriate to mark resolution)\n#\n#       both modified:      foo.txt\n\nand the big difference between 'git status' and the others is that\nit's legitimate to run it while resolving conflicts, while calling\n'git merge' or 'git commit' can be done only by mistake.\n\nIt's not serious to eat 3 or 4 lines of the screen to display a\nmessage to tell the user he shouldn't have done this, but it'd be\ndisturbing to eat more than 1 line for a common case.\n\n> I think we should consolidate them all.\n\nRight, although \"commit\" is definitely the most important (dumb users\ndon't need \"git merge\").\n\n>  - Decide if we go one-step (i.e. just say \"resolve conflicts\" and nothing\n>    else) or two-step (i.e. say \"Fix them up in the work tree\" and \"mark\n>    them resolved in the index\") approach; I am leaning toward the latter.\n\nMeetoo.\n\n>  - Decide the exact language used.  I think \"Fix them up in the work tree\"\n>    is passably okay, but \"Mark them resolved in the index\" needs to be\n>    made more concrete, and \"git add <filename>\" is not quite it, I am\n>    afraid.\n\nSee above. To me, pointing to \"git commit -a\" and \"git add\" is\nsufficient.\n\nPointing to \"git commit -a\" is also important to me, because Git\nnewbies may have been told to always use \"git commit\" with \"-a\"\n(common use-case: \"I have to use Git, I know SVN and I don't want to\nlearn anything new\").\n\n>  - Omit issuing the advisory message when advice.resolveConflict is set to\n>    false.\n\nSensible, yes.\n\n>  - Perhaps have a common helper function to do this from the commands that\n>    need to give the advice.\n\nProbably not, because the advice will be different:\n\ngit merge => please resolve and commit before you can merge\ngit commit => please resolve before you can commit\n...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"130934","messageId":"7v8wcbw14f.fsf@alter.siamese.dyndns.org","threadId":"22103","inReplyTo":"vpqtyuz86ny.fsf@bauges.imag.fr","subject":"Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-06T18:15:44Z","receivedAt":"2010-01-06T18:15:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Maybe \"sometimes\", but to me, that's sufficiently rare to be omited\n> here (I don't think I ever used 'git rm' to resolve a conflict). The\n> user manual says this:\n>\n> ,----\n> | Each time you resolve the conflicts in a file and update the index:\n> | \n> | $ git add file.txt\n> | \n> | the different stages of that file will be \"collapsed\", after which git\n> | diff will (by default) no longer show diffs for that file.\n> `----\n>\n> and I don't think it makes sense to try to be more exhaustive here\n> than in the user-manual.\n\nI tend to disagree.  The user-manual, as it currently stands, is an\nintroductory document that covers the concepts and the surface of git.\nIt's purpose is not to replace the main manual, but to give the necessary\nfoundation _before_ people read the manual.\n\n>> The need to give this advice on how to resolve conflicts is shared among\n>> commands like 'git merge', 'git cherry-pick', and 'git status', to name a\n>> few.\n>\n> Not sure 'status' needs anything more. It already says\n>\n> # Unmerged paths:\n> #   (use \"git add/rm <file>...\" as appropriate to mark resolution)\n> #\n> #       both modified:      foo.txt\n\nThis message was exactly what I was getting at.  The message in the\nparentheses is what I called \"advice\".  As I was suggesting to share the\nadvice messages used by commands that deal with mergy situations (like the\none you added in your patch), looking at existing examples to find the\nbest one and using that in other places would be the most effective\napproach.  That one line we see above is concise and does mention \"rm\" as\nwell.  Why not use it?\n\n>> I think we should consolidate them all.\n>\n> Right, although \"commit\" is definitely the most important (dumb users\n> don't need \"git merge\").\n\nYour \"dumb users\" don't get the unmerged error from commit, either, if\nthey don't need \"git merge\".\n"},{"id":"130943","messageId":"vpqocl76mdo.fsf@bauges.imag.fr","threadId":"22103","inReplyTo":"7v8wcbw14f.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-01-06T19:53:23Z","receivedAt":"2010-01-06T19:53:23Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> # Unmerged paths:\n>> #   (use \"git add/rm <file>...\" as appropriate to mark resolution)\n[...]\n>  That one line we see above is concise and does mention \"rm\" as\n> well.  Why not use it?\n\nFine, you convinced me. I was thinking of a longer sentence, but \"/rm\"\ndoesn't harm.\n\n>>> I think we should consolidate them all.\n>>\n>> Right, although \"commit\" is definitely the most important (dumb users\n>> don't need \"git merge\").\n>\n> Your \"dumb users\" don't get the unmerged error from commit, either, if\n> they don't need \"git merge\".\n\nThey'd use \"pull\", not merge. Anyway, I did it for commit, merge,\npull, revert, cherry-pick. I guess we covered the common cases.\nThe patch seems to have a lot of redundancies, but I think trying to\nfactor this into helper functions would be much more effort than the\nfew cut-and-paste that I had to do, since each instance is a slight\nvariant of each other ...\n\nPatch follows. Let me know in case you prefer to split it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"130945","messageId":"7vy6kbt2ww.fsf@alter.siamese.dyndns.org","threadId":"22103","inReplyTo":"vpqocl76mdo.fsf@bauges.imag.fr","subject":"Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-06T20:05:19Z","receivedAt":"2010-01-06T20:05:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> They'd use \"pull\", not merge. Anyway, I did it for commit, merge,\n> pull, revert, cherry-pick. I guess we covered the common cases.\n> The patch seems to have a lot of redundancies, but I think trying to\n> factor this into helper functions would be much more effort than the\n> few cut-and-paste that I had to do, since each instance is a slight\n> variant of each other ...\n\nI'd be more worried about longer term maintainability than one time\nexpediency of producing your single patch to add these messages.  If the\nmessages are cast in stone, we can just verify they are consistent _now_\nand forget about them, but I suspect not even you are perfect to predict\nthat we won't come up with different/better ways to resolve and mark them\nresolved in the future and write a set of messages that won't have to\nchange.\n"},{"id":"130948","messageId":"vpqhbqz6lcr.fsf@bauges.imag.fr","threadId":"22103","inReplyTo":"7vy6kbt2ww.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-01-06T20:15:32Z","receivedAt":"2010-01-06T20:15:32Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> They'd use \"pull\", not merge. Anyway, I did it for commit, merge,\n>> pull, revert, cherry-pick. I guess we covered the common cases.\n>> The patch seems to have a lot of redundancies, but I think trying to\n>> factor this into helper functions would be much more effort than the\n>> few cut-and-paste that I had to do, since each instance is a slight\n>> variant of each other ...\n>\n> I'd be more worried about longer term maintainability than one time\n> expediency of producing your single patch to add these messages.  If the\n> messages are cast in stone, we can just verify they are consistent _now_\n> and forget about them, but I suspect not even you are perfect to predict\n> that we won't come up with different/better ways to resolve and mark them\n> resolved in the future and write a set of messages that won't have to\n> change.\n\nSorry, I sent the message a bit too early. Re-reading the patch, there\nwere actually pieces to factor. There's still duplication between C\nand shell, and sentences which are actually reworded from a place to\nanother.\n\nOTOH, maybe this reveals some differences that should be eliminated.\nFor example, 'git cherry-pick' has no problem with\n$GIT_DIR/MERGE_HEAD, while 'git merge' will immediately if it exists.\nMaybe we should write a helper like\nensure_everything_is_ok_for_a_merge_or_die() called by both\ncherry-pick and merge?\n\nThis time, patch actually follows ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"130949","messageId":"1262809074-27137-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22103","inReplyTo":"vpqhbqz6lcr.fsf@bauges.imag.fr","subject":"[PATCH v2] Be more user-friendly when refusing to do something because of conflict.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-06T20:17:54Z","receivedAt":"2010-01-06T20:17:54Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Various commands refuse to run in the presence of conflicts (commit,\nmerge, pull, cherry-pick/revert). They all used to provide rough, and\ninconsistant error messages.\n\nA new variable advice.resolveconflict is introduced, and allows more\nverbose messages, pointing the user to the appropriate solution.\n\nFor commit, the error message used to look like this:\n\n$ git commit\nfoo.txt: needs merge\nfoo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\nfoo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\nfoo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\nerror: Error building trees\n\nThe \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\noption to make the output more consistant with the other porcelain\ncommands, and catch the error in return, to stop with a clean error\nmessage. The next lines were displayed by a call to cache_tree_update(),\nwhich is not reached anymore if we noticed the conflict.\n\nPull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD\nexists instead of waiting for merge to complain.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Documentation/config.txt |    4 ++++\n advice.c                 |   12 ++++++++++++\n advice.h                 |    3 +++\n builtin-commit.c         |   15 +++++++++++++--\n builtin-merge.c          |   15 ++++++++++-----\n builtin-revert.c         |   15 ++++++++++++++-\n git-pull.sh              |   25 +++++++++++++++++++++++--\n 7 files changed, 79 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 23a965e..5078d26 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -130,6 +130,10 @@ advice.*::\n \t\tAdvice shown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwritting local changes.\n \t\tDefault: true.\n+\tresolveConflict::\n+\t\tAdvices shown by various commands when conflicts\n+\t\tprevent the operation from being performed. \n+\t\tDefault: true.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex cb666ac..ec2bd82 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -3,6 +3,7 @@\n int advice_push_nonfastforward = 1;\n int advice_status_hints = 1;\n int advice_commit_before_merge = 1;\n+int advice_resolve_conflict = 1;\n \n static struct {\n \tconst char *name;\n@@ -11,6 +12,7 @@ static struct {\n \t{ \"pushnonfastforward\", &advice_push_nonfastforward },\n \t{ \"statushints\", &advice_status_hints },\n \t{ \"commitbeforemerge\", &advice_commit_before_merge },\n+\t{ \"resolveconflict\", &advice_resolve_conflict },\n };\n \n int git_default_advice_config(const char *var, const char *value)\n@@ -27,3 +29,13 @@ int git_default_advice_config(const char *var, const char *value)\n \n \treturn 0;\n }\n+\n+void NORETURN die_resolve_conflict(const char *me)\n+{\n+\tif (advice_resolve_conflict)\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n+\t\t    \"Please, fix them up in the work tree, and then use 'git add/rm <file>'\\n\"\n+\t\t    \"as appropriate to mark resolution, or use 'git commit -a'.\", me);\n+\telse\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n+}\ndiff --git a/advice.h b/advice.h\nindex 3de5000..a6b4422 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -4,7 +4,10 @@\n extern int advice_push_nonfastforward;\n extern int advice_status_hints;\n extern int advice_commit_before_merge;\n+extern int advice_resolve_conflict;\n \n int git_default_advice_config(const char *var, const char *value);\n \n+extern void NORETURN die_resolve_conflict(const char *me);\n+\n #endif /* ADVICE_H */\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 3dfcd77..a4977ac 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -235,6 +235,17 @@ static void create_base_index(void)\n \t\texit(128); /* We've already reported the error, finish dying */\n }\n \n+static void refresh_cache_or_die(int refresh_flags)\n+{\n+\t/*\n+\t * refresh_flags contains REFRESH_QUIET, so the only errors\n+\t * are for unmerged entries.\n+\t*/\n+\tif(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {\n+\t\tdie_resolve_conflict(\"commit\");\n+\t}\n+}\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)\n {\n \tint fd;\n@@ -274,7 +285,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (all || (also && pathspec && *pathspec)) {\n \t\tint fd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n@@ -293,7 +304,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex f1c84d7..abe6c03 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -847,11 +847,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n \tstruct commit_list **remotes = &remoteheads;\n \n-\tif (file_exists(git_path(\"MERGE_HEAD\")))\n-\t\tdie(\"You have not concluded your merge. (MERGE_HEAD exists)\");\n-\tif (read_cache_unmerged())\n-\t\tdie(\"You are in the middle of a conflicted merge.\"\n-\t\t\t\t\" (index unmerged)\");\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(\"merge\");\n+\t}\n+\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n+\t\tif (advice_resolve_conflict)\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n+\t\t\t    \"Please, commit your changes before you can merge.\");\n+\t\telse\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n+\t}\n \n \t/*\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 151aa6a..d14dde3 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)\n \treturn tree;\n }\n \n+static NORETURN void die_dirty_index(const char *me)\n+{\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(me);\n+\t} else {\n+\t\tif (advice_commit_before_merge)\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\"\n+\t\t\t    \"Please, commit your changes or stash them to proceed.\", me);\n+\t\telse\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\", me);\n+\t}\n+}\n+\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\tif (get_sha1(\"HEAD\", head))\n \t\t\tdie (\"You do not have a valid HEAD\");\n \t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie (\"Dirty index: cannot %s\", me);\n+\t\t\tdie_dirty_index(me);\n \t}\n \tdiscard_cache();\n \ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9e69ada..54ce0af 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -13,8 +13,29 @@ set_reflog_action \"pull $*\"\n require_work_tree\n cd_to_toplevel\n \n-test -z \"$(git ls-files -u)\" ||\n-\tdie \"You are in the middle of a conflicted merge.\"\n+\n+die_conflict () {\n+    git diff-index --cached --name-status -r --ignore-submodules HEAD --\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"Pull is not possible because you have unmerged files.\n+Please, fix them up in the work tree, and then use 'git add/rm <file>'\n+as appropriate to mark resolution, or use 'git commit -a'.\"\n+    else\n+\tdie \"Pull is not possible because you have unmerged files.\"\n+    fi\n+}\n+\n+die_merge () {\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\n+Please, commit your changes before you can merge.\"\n+    else\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\"\n+    fi\n+}\n+\n+test -z \"$(git ls-files -u)\" || die_conflict\n+test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity=\n-- \n1.6.6.81.g35ec3e.dirty\n"},{"id":"130950","messageId":"1262810185-7046-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22103","inReplyTo":"1262809074-27137-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v3] Be more user-friendly when refusing to do something because of conflict.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-06T20:36:25Z","receivedAt":"2010-01-06T20:36:25Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Various commands refuse to run in the presence of conflicts (commit,\nmerge, pull, cherry-pick/revert). They all used to provide rough, and\ninconsistant error messages.\n\nA new variable advice.resolveconflict is introduced, and allows more\nverbose messages, pointing the user to the appropriate solution.\n\nFor commit, the error message used to look like this:\n\n$ git commit\nfoo.txt: needs merge\nfoo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\nfoo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\nfoo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\nerror: Error building trees\n\nThe \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\noption to make the output more consistant with the other porcelain\ncommands, and catch the error in return, to stop with a clean error\nmessage. The next lines were displayed by a call to cache_tree_update(),\nwhich is not reached anymore if we noticed the conflict.\n\nPull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD\nexists instead of waiting for merge to complain.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nSorry, v2 missed a #include \"git-compat-utils.h\", it's not even compilable.\n\n Documentation/config.txt |    4 ++++\n advice.c                 |   12 ++++++++++++\n advice.h                 |    5 +++++\n builtin-commit.c         |   15 +++++++++++++--\n builtin-merge.c          |   15 ++++++++++-----\n builtin-revert.c         |   15 ++++++++++++++-\n git-pull.sh              |   25 +++++++++++++++++++++++--\n 7 files changed, 81 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 23a965e..5078d26 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -130,6 +130,10 @@ advice.*::\n \t\tAdvice shown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwritting local changes.\n \t\tDefault: true.\n+\tresolveConflict::\n+\t\tAdvices shown by various commands when conflicts\n+\t\tprevent the operation from being performed. \n+\t\tDefault: true.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex cb666ac..ec2bd82 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -3,6 +3,7 @@\n int advice_push_nonfastforward = 1;\n int advice_status_hints = 1;\n int advice_commit_before_merge = 1;\n+int advice_resolve_conflict = 1;\n \n static struct {\n \tconst char *name;\n@@ -11,6 +12,7 @@ static struct {\n \t{ \"pushnonfastforward\", &advice_push_nonfastforward },\n \t{ \"statushints\", &advice_status_hints },\n \t{ \"commitbeforemerge\", &advice_commit_before_merge },\n+\t{ \"resolveconflict\", &advice_resolve_conflict },\n };\n \n int git_default_advice_config(const char *var, const char *value)\n@@ -27,3 +29,13 @@ int git_default_advice_config(const char *var, const char *value)\n \n \treturn 0;\n }\n+\n+void NORETURN die_resolve_conflict(const char *me)\n+{\n+\tif (advice_resolve_conflict)\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n+\t\t    \"Please, fix them up in the work tree, and then use 'git add/rm <file>'\\n\"\n+\t\t    \"as appropriate to mark resolution, or use 'git commit -a'.\", me);\n+\telse\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n+}\ndiff --git a/advice.h b/advice.h\nindex 3de5000..acd5fdd 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -1,10 +1,15 @@\n #ifndef ADVICE_H\n #define ADVICE_H\n \n+#include \"git-compat-util.h\"\n+\n extern int advice_push_nonfastforward;\n extern int advice_status_hints;\n extern int advice_commit_before_merge;\n+extern int advice_resolve_conflict;\n \n int git_default_advice_config(const char *var, const char *value);\n \n+extern void NORETURN die_resolve_conflict(const char *me);\n+\n #endif /* ADVICE_H */\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 3dfcd77..a4977ac 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -235,6 +235,17 @@ static void create_base_index(void)\n \t\texit(128); /* We've already reported the error, finish dying */\n }\n \n+static void refresh_cache_or_die(int refresh_flags)\n+{\n+\t/*\n+\t * refresh_flags contains REFRESH_QUIET, so the only errors\n+\t * are for unmerged entries.\n+\t*/\n+\tif(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {\n+\t\tdie_resolve_conflict(\"commit\");\n+\t}\n+}\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)\n {\n \tint fd;\n@@ -274,7 +285,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (all || (also && pathspec && *pathspec)) {\n \t\tint fd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n@@ -293,7 +304,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex f1c84d7..abe6c03 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -847,11 +847,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n \tstruct commit_list **remotes = &remoteheads;\n \n-\tif (file_exists(git_path(\"MERGE_HEAD\")))\n-\t\tdie(\"You have not concluded your merge. (MERGE_HEAD exists)\");\n-\tif (read_cache_unmerged())\n-\t\tdie(\"You are in the middle of a conflicted merge.\"\n-\t\t\t\t\" (index unmerged)\");\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(\"merge\");\n+\t}\n+\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n+\t\tif (advice_resolve_conflict)\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n+\t\t\t    \"Please, commit your changes before you can merge.\");\n+\t\telse\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n+\t}\n \n \t/*\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 151aa6a..d14dde3 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)\n \treturn tree;\n }\n \n+static NORETURN void die_dirty_index(const char *me)\n+{\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(me);\n+\t} else {\n+\t\tif (advice_commit_before_merge)\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\"\n+\t\t\t    \"Please, commit your changes or stash them to proceed.\", me);\n+\t\telse\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\", me);\n+\t}\n+}\n+\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\tif (get_sha1(\"HEAD\", head))\n \t\t\tdie (\"You do not have a valid HEAD\");\n \t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie (\"Dirty index: cannot %s\", me);\n+\t\t\tdie_dirty_index(me);\n \t}\n \tdiscard_cache();\n \ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9e69ada..54ce0af 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -13,8 +13,29 @@ set_reflog_action \"pull $*\"\n require_work_tree\n cd_to_toplevel\n \n-test -z \"$(git ls-files -u)\" ||\n-\tdie \"You are in the middle of a conflicted merge.\"\n+\n+die_conflict () {\n+    git diff-index --cached --name-status -r --ignore-submodules HEAD --\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"Pull is not possible because you have unmerged files.\n+Please, fix them up in the work tree, and then use 'git add/rm <file>'\n+as appropriate to mark resolution, or use 'git commit -a'.\"\n+    else\n+\tdie \"Pull is not possible because you have unmerged files.\"\n+    fi\n+}\n+\n+die_merge () {\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\n+Please, commit your changes before you can merge.\"\n+    else\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\"\n+    fi\n+}\n+\n+test -z \"$(git ls-files -u)\" || die_conflict\n+test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity=\n-- \n1.6.6.81.g2955.dirty\n"},{"id":"130951","messageId":"7vskajq716.fsf@alter.siamese.dyndns.org","threadId":"22103","inReplyTo":"1262809074-27137-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v2] Be more user-friendly when refusing to do something because of conflict.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-06T21:04:37Z","receivedAt":"2010-01-06T21:04:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Various commands refuse to run in the presence of conflicts (commit,\n> merge, pull, cherry-pick/revert). They all used to provide rough, and\n> inconsistant error messages.\n>\n> A new variable advice.resolveconflict is introduced, and allows more\n> verbose messages, pointing the user to the appropriate solution.\n>\n> For commit, the error message used to look like this:\n>\n> $ git commit\n> foo.txt: needs merge\n> foo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\n> foo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\n> foo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\n> error: Error building trees\n>\n> The \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\n> option to make the output more consistant with the other porcelain\n> commands, and catch the error in return, to stop with a clean error\n> message. The next lines were displayed by a call to cache_tree_update(),\n> which is not reached anymore if we noticed the conflict.\n\n\"The new output looks like this, which is much better...\" is missing\nhere.\n\n> @@ -27,3 +29,13 @@ int git_default_advice_config(const char *var, const char *value)\n>  \n>  \treturn 0;\n>  }\n> +\n> +void NORETURN die_resolve_conflict(const char *me)\n> +{\n> +\tif (advice_resolve_conflict)\n> +\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n> +\t\t    \"Please, fix them up in the work tree, and then use 'git add/rm <file>'\\n\"\n> +\t\t    \"as appropriate to mark resolution, or use 'git commit -a'.\", me);\n> +\telse\n> +\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n> +}\n\nNice, but the advice contrasts between squirrels and oranges.\n\nOne advice, \"use add/rm as appropriate to mark resolution\" goes only as\nfar as making the index in order, without recording it in a commit.  The\nother, \"commit -a\", will make a commit, I suspect that \"commit -a\" needs\nto be matched with \"commit\" if the user chooses to take the first advice\nof resolving paths incrementally.  IOW\n\n    use 'git add/rm <file>' as appropriate to mark resolution and make a\n    commit, or use 'commit -a', before running me again.\n\nmight be more appropriate.\n\n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index 3dfcd77..a4977ac 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -235,6 +235,17 @@ static void create_base_index(void)\n>  \t\texit(128); /* We've already reported the error, finish dying */\n>  }\n>  \n> +static void refresh_cache_or_die(int refresh_flags)\n> +{\n> +\t/*\n> +\t * refresh_flags contains REFRESH_QUIET, so the only errors\n> +\t * are for unmerged entries.\n> +\t*/\n\nMixed indentation.\n\n> +\tif(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {\n\nSP after \"if\".\n\nWhat should we see upon \"commit --dry-run\" and what does the code after\nthe patch produce?  Are we sure refresh_flags always lack REFRESH_UNMERGED\nthat allows unmerged entries and produces the unmerged error messages when\nneeded?\n\n> diff --git a/builtin-merge.c b/builtin-merge.c\n> index f1c84d7..abe6c03 100644\n> --- a/builtin-merge.c\n> +++ b/builtin-merge.c\n> @@ -847,11 +847,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>  \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n>  \tstruct commit_list **remotes = &remoteheads;\n>  \n> -\tif (file_exists(git_path(\"MERGE_HEAD\")))\n> -\t\tdie(\"You have not concluded your merge. (MERGE_HEAD exists)\");\n> -\tif (read_cache_unmerged())\n> -\t\tdie(\"You are in the middle of a conflicted merge.\"\n> -\t\t\t\t\" (index unmerged)\");\n> +\tif (read_cache_unmerged()) {\n> +\t\tdie_resolve_conflict(\"merge\");\n> +\t}\n> +\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n> +\t\tif (advice_resolve_conflict)\n> +\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n> +\t\t\t    \"Please, commit your changes before you can merge.\");\n> +\t\telse\n> +\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n> +\t}\n\nIt is not a very big deal, but why are these checked in different order\nafter the patch?  Unless the new order is justifiably better, I'd rather\nsee the order kept as before.  Otherwise please justify it in the proposed\ncommit log message.\n\nNote that the user might have already run \"add -u\" to mark everything\nresolved, in which case MERGE_HEAD will still exist even though the index\nis free of ummerged entries.\n\n> diff --git a/builtin-revert.c b/builtin-revert.c\n\nNice.\n\n> diff --git a/git-pull.sh b/git-pull.sh\n> index 9e69ada..54ce0af 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -13,8 +13,29 @@ set_reflog_action \"pull $*\"\n>  require_work_tree\n>  cd_to_toplevel\n>  \n> -test -z \"$(git ls-files -u)\" ||\n> -\tdie \"You are in the middle of a conflicted merge.\"\n> +\n> +die_conflict () {\n> +    git diff-index --cached --name-status -r --ignore-submodules HEAD --\n> +    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n> +\tdie \"Pull is not possible because you have unmerged files.\n> +Please, fix them up in the work tree, and then use 'git add/rm <file>'\n> +as appropriate to mark resolution, or use 'git commit -a'.\"\n> +    else\n> +\tdie \"Pull is not possible because you have unmerged files.\"\n> +    fi\n> +}\n> +\n> +die_merge () {\n> +    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n> +\tdie \"You have not concluded your merge (MERGE_HEAD exists).\n> +Please, commit your changes before you can merge.\"\n> +    else\n> +\tdie \"You have not concluded your merge (MERGE_HEAD exists).\"\n> +    fi\n> +}\n> +\n> +test -z \"$(git ls-files -u)\" || die_conflict\n> +test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n\nNice.  Maybe we want to handle other cases like:\n\n\t$ git rebase master\n        ... conflicted\n        ... called away for a 30-minutes meeting\n        ... forgot the user was in the middle of the rebase\n        $ git pull\n\nand the \"pull\" refused to run because the earlier \"rebase\" hasn't been\nconcluded (I suspect an earlier \"am\" failure would be the same issue).\n\nThanks.\n"},{"id":"131000","messageId":"vpqiqbeornm.fsf@bauges.imag.fr","threadId":"22103","inReplyTo":"7vskajq716.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Be more user-friendly when refusing to do something because of conflict.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-01-07T15:34:21Z","receivedAt":"2010-01-07T15:34:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Thanks for your detailed feedback.\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> \"The new output looks like this, which is much better...\" is missing\n> here.\n\nAdded.\n\n> One advice, \"use add/rm as appropriate to mark resolution\" goes only as\n> far as making the index in order, without recording it in a commit.  The\n> other, \"commit -a\", will make a commit, I suspect that \"commit -a\" needs\n> to be matched with \"commit\" if the user chooses to take the first advice\n> of resolving paths incrementally.  IOW\n>\n>     use 'git add/rm <file>' as appropriate to mark resolution and make a\n>     commit, or use 'commit -a', before running me again.\n>\n> might be more appropriate.\n\nApplied.\n\nThe same sentence has a slightly different meaning for commit and\nmerge. For commit \"make a commit\" means \"redo what you tried to do\",\nand for merge, it means \"make a commit before you redo what you tried\nto do\".\n\n>> +static void refresh_cache_or_die(int refresh_flags)\n>> +{\n>> +\t/*\n>> +\t * refresh_flags contains REFRESH_QUIET, so the only errors\n>> +\t * are for unmerged entries.\n>> +\t*/\n>\n> Mixed indentation.\n\nfixed.\n\n>> +\tif(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {\n>\n> SP after \"if\".\n\nfixed (together with the useless brace which you had missed ;-) ).\n\n> What should we see upon \"commit --dry-run\" and what does the code after\n> the patch produce?\n\nGood remark. Just tried, it works without option, with -a, -o, -i.\nThey all give the usual status output, except -o which complains:\n\n$ git commit --dry-run -o foo.txt\nfatal: cannot do a partial commit during a merge.\n\n(as before)\n\n> Are we sure refresh_flags always lack REFRESH_UNMERGED that allows\n> unmerged entries and produces the unmerged error messages when\n> needed?\n\nArgument from intimidation:\nIf they hadn't, we would have noticed the output (foo.txt: needs\nmerge) before the patch.\n\nRational argument:\nprepare_index starts with this:\n\n\tif (is_status)\n\t\trefresh_flags |= REFRESH_UNMERGED;\n\nand each call to refresh_cache_or_die is within prepare_index, with\nthese flags, untouched.\n\n>> -\tif (file_exists(git_path(\"MERGE_HEAD\"))) [...]\n>> -\tif (read_cache_unmerged()) [...]\n>> +\tif (read_cache_unmerged()) {\n>> +\t\tdie_resolve_conflict(\"merge\");\n>> +\t}\n>> +\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n>> +\t\tif (advice_resolve_conflict)\n>> +\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n>> +\t\t\t    \"Please, commit your changes before you can merge.\");\n>> +\t\telse\n>> +\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n>> +\t}\n>\n> It is not a very big deal, but why are these checked in different order\n> after the patch?\n[...]\n> Note that the user might have already run \"add -u\" to mark everything\n> resolved, in which case MERGE_HEAD will still exist even though the index\n> is free of ummerged entries.\n\nThat's precisely the reason. You hardly have conflicts without a\nMERGE_HEAD, but it's sensible to have a MERGE_HEAD without unmerged\nentries in the index, which means you're one step closer to the\ncommit.\n\nI've added a comment (not a commit message) to make it clear.\n\n> Nice.  Maybe we want to handle other cases like:\n>\n> \t$ git rebase master\n>         ... conflicted\n\n(if it's conflicted, it's OK, same user-friendly message. If you\nalready git add-ed your conflicts, git pull will run normally :-( ).\n\n>         ... called away for a 30-minutes meeting\n>         ... forgot the user was in the middle of the rebase\n>         $ git pull\n>\n> and the \"pull\" refused to run because the earlier \"rebase\" hasn't been\n> concluded (I suspect an earlier \"am\" failure would be the same issue).\n\nYes, but I think that's a separate topic (and I'm getting short in\ntime budget). This will also apply to many commands (pull,\nmerge, ...), including shell and C. It would probably be good to have\na C function like can_I_start_merge() exposed as a plumbing command to\nbe used by git-pull.sh.\n\nAnd I'm wondering whether the ability to run pull in the middle of a\nrebase is a bug or a feature. Never did that, but I can imagine\nsomeone doing a 'rebase -i' with an 'edit' command to let rebase stop,\ndo a merge, and then 'rebase --continue', to insert a merge commit in\nthe middle of a patch serie.\n\nPatch follows. Fyi the difference with v3 are:\n\ndiff --git a/advice.c b/advice.c\nindex ec2bd82..3309521 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -33,9 +33,13 @@ int git_default_advice_config(const char *var, const char *value)\n void NORETURN die_resolve_conflict(const char *me)\n {\n        if (advice_resolve_conflict)\n+               /*\n+                * Message used both when 'git commit' fails and when\n+                * other commands doing a merge do.\n+                */\n                die(\"'%s' is not possible because you have unmerged files.\\n\"\n-                   \"Please, fix them up in the work tree, and then use 'git add/rm <file>'\\n\"\n-                   \"as appropriate to mark resolution, or use 'git commit -a'.\", me);\n+                   \"Please, fix them up in the work tree, and then use 'git add/rm <file>' as\\n\"\n+                   \"appropriate to mark resolution and make a commit, or use 'git commit -a'.\", me);\n        else\n                die(\"'%s' is not possible because you have unmerged files.\", me);\n }\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex a4977ac..b3b37c2 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -240,8 +240,8 @@ static void refresh_cache_or_die(int refresh_flags)\n        /*\n         * refresh_flags contains REFRESH_QUIET, so the only errors\n         * are for unmerged entries.\n-       */\n-       if(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {\n+        */\n+       if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {\n                die_resolve_conflict(\"commit\");\n        }\n }\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex abe6c03..79a35c3 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -851,6 +851,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n                die_resolve_conflict(\"merge\");\n        }\n        if (file_exists(git_path(\"MERGE_HEAD\"))) {\n+               /*\n+                * There is no unmerged entry, don't advise 'git\n+                * add/rm <file>', just 'git commit'.\n+                */\n                if (advice_resolve_conflict)\n                        die(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n                            \"Please, commit your changes before you can merge.\");\n\n--\nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"131001","messageId":"1262878552-4775-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22103","inReplyTo":"vpqiqbeornm.fsf@bauges.imag.fr","subject":"[PATCH v4] Be more user-friendly when refusing to do something because of conflict.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-07T15:35:52Z","receivedAt":"2010-01-07T15:35:52Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Various commands refuse to run in the presence of conflicts (commit,\nmerge, pull, cherry-pick/revert). They all used to provide rough, and\ninconsistant error messages.\n\nA new variable advice.resolveconflict is introduced, and allows more\nverbose messages, pointing the user to the appropriate solution.\n\nFor commit, the error message used to look like this:\n\n$ git commit\nfoo.txt: needs merge\nfoo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\nfoo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\nfoo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\nerror: Error building trees\n\nThe \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\noption to make the output more consistant with the other porcelain\ncommands, and catch the error in return, to stop with a clean error\nmessage. The next lines were displayed by a call to cache_tree_update(),\nwhich is not reached anymore if we noticed the conflict.\n\nThe new output looks like:\n\nU       foo.txt\nfatal: 'commit' is not possible because you have unmerged files.\nPlease, fix them up in the work tree, and then use 'git add/rm <file>' as\nappropriate to mark resolution and make a commit, or use 'git commit -a'.\n\nPull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD\nexists instead of waiting for merge to complain.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Documentation/config.txt |    4 ++++\n advice.c                 |   16 ++++++++++++++++\n advice.h                 |    5 +++++\n builtin-commit.c         |   14 ++++++++++++--\n builtin-merge.c          |   19 ++++++++++++++-----\n builtin-revert.c         |   15 ++++++++++++++-\n git-pull.sh              |   25 +++++++++++++++++++++++--\n 7 files changed, 88 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 23a965e..5078d26 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -130,6 +130,10 @@ advice.*::\n \t\tAdvice shown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwritting local changes.\n \t\tDefault: true.\n+\tresolveConflict::\n+\t\tAdvices shown by various commands when conflicts\n+\t\tprevent the operation from being performed. \n+\t\tDefault: true.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex cb666ac..3309521 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -3,6 +3,7 @@\n int advice_push_nonfastforward = 1;\n int advice_status_hints = 1;\n int advice_commit_before_merge = 1;\n+int advice_resolve_conflict = 1;\n \n static struct {\n \tconst char *name;\n@@ -11,6 +12,7 @@ static struct {\n \t{ \"pushnonfastforward\", &advice_push_nonfastforward },\n \t{ \"statushints\", &advice_status_hints },\n \t{ \"commitbeforemerge\", &advice_commit_before_merge },\n+\t{ \"resolveconflict\", &advice_resolve_conflict },\n };\n \n int git_default_advice_config(const char *var, const char *value)\n@@ -27,3 +29,17 @@ int git_default_advice_config(const char *var, const char *value)\n \n \treturn 0;\n }\n+\n+void NORETURN die_resolve_conflict(const char *me)\n+{\n+\tif (advice_resolve_conflict)\n+\t\t/*\n+\t\t * Message used both when 'git commit' fails and when\n+\t\t * other commands doing a merge do.\n+\t\t */\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n+\t\t    \"Please, fix them up in the work tree, and then use 'git add/rm <file>' as\\n\"\n+\t\t    \"appropriate to mark resolution and make a commit, or use 'git commit -a'.\", me);\n+\telse\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n+}\ndiff --git a/advice.h b/advice.h\nindex 3de5000..acd5fdd 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -1,10 +1,15 @@\n #ifndef ADVICE_H\n #define ADVICE_H\n \n+#include \"git-compat-util.h\"\n+\n extern int advice_push_nonfastforward;\n extern int advice_status_hints;\n extern int advice_commit_before_merge;\n+extern int advice_resolve_conflict;\n \n int git_default_advice_config(const char *var, const char *value);\n \n+extern void NORETURN die_resolve_conflict(const char *me);\n+\n #endif /* ADVICE_H */\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 3dfcd77..6075a59 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -235,6 +235,16 @@ static void create_base_index(void)\n \t\texit(128); /* We've already reported the error, finish dying */\n }\n \n+static void refresh_cache_or_die(int refresh_flags)\n+{\n+\t/*\n+\t * refresh_flags contains REFRESH_QUIET, so the only errors\n+\t * are for unmerged entries.\n+\t */\n+\tif (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))\n+\t\tdie_resolve_conflict(\"commit\");\n+}\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)\n {\n \tint fd;\n@@ -274,7 +284,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (all || (also && pathspec && *pathspec)) {\n \t\tint fd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n@@ -293,7 +303,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex f1c84d7..79a35c3 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -847,11 +847,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n \tstruct commit_list **remotes = &remoteheads;\n \n-\tif (file_exists(git_path(\"MERGE_HEAD\")))\n-\t\tdie(\"You have not concluded your merge. (MERGE_HEAD exists)\");\n-\tif (read_cache_unmerged())\n-\t\tdie(\"You are in the middle of a conflicted merge.\"\n-\t\t\t\t\" (index unmerged)\");\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(\"merge\");\n+\t}\n+\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n+\t\t/*\n+\t\t * There is no unmerged entry, don't advise 'git\n+\t\t * add/rm <file>', just 'git commit'.\n+\t\t */\n+\t\tif (advice_resolve_conflict)\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n+\t\t\t    \"Please, commit your changes before you can merge.\");\n+\t\telse\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n+\t}\n \n \t/*\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 151aa6a..d14dde3 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)\n \treturn tree;\n }\n \n+static NORETURN void die_dirty_index(const char *me)\n+{\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(me);\n+\t} else {\n+\t\tif (advice_commit_before_merge)\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\"\n+\t\t\t    \"Please, commit your changes or stash them to proceed.\", me);\n+\t\telse\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\", me);\n+\t}\n+}\n+\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\tif (get_sha1(\"HEAD\", head))\n \t\t\tdie (\"You do not have a valid HEAD\");\n \t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie (\"Dirty index: cannot %s\", me);\n+\t\t\tdie_dirty_index(me);\n \t}\n \tdiscard_cache();\n \ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9e69ada..54ce0af 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -13,8 +13,29 @@ set_reflog_action \"pull $*\"\n require_work_tree\n cd_to_toplevel\n \n-test -z \"$(git ls-files -u)\" ||\n-\tdie \"You are in the middle of a conflicted merge.\"\n+\n+die_conflict () {\n+    git diff-index --cached --name-status -r --ignore-submodules HEAD --\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"Pull is not possible because you have unmerged files.\n+Please, fix them up in the work tree, and then use 'git add/rm <file>'\n+as appropriate to mark resolution, or use 'git commit -a'.\"\n+    else\n+\tdie \"Pull is not possible because you have unmerged files.\"\n+    fi\n+}\n+\n+die_merge () {\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\n+Please, commit your changes before you can merge.\"\n+    else\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\"\n+    fi\n+}\n+\n+test -z \"$(git ls-files -u)\" || die_conflict\n+test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity=\n-- \n1.6.6.81.g872e.dirty\n"},{"id":"131027","messageId":"1262887810-19415-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"22103","inReplyTo":"1262878552-4775-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v5] Be more user-friendly when refusing to do something because of conflict.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-01-07T18:10:10Z","receivedAt":"2010-01-07T18:10:10Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Various commands refuse to run in the presence of conflicts (commit,\nmerge, pull, cherry-pick/revert). They all used to provide rough, and\ninconsistant error messages.\n\nA new variable advice.resolveconflict is introduced, and allows more\nverbose messages, pointing the user to the appropriate solution.\n\nFor commit, the error message used to look like this:\n\n$ git commit\nfoo.txt: needs merge\nfoo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)\nfoo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)\nfoo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)\nerror: Error building trees\n\nThe \"need merge\" line is given by refresh_cache. We add the IN_PORCELAIN\noption to make the output more consistant with the other porcelain\ncommands, and catch the error in return, to stop with a clean error\nmessage. The next lines were displayed by a call to cache_tree_update(),\nwhich is not reached anymore if we noticed the conflict.\n\nThe new output looks like:\n\nU       foo.txt\nfatal: 'commit' is not possible because you have unmerged files.\nPlease, fix them up in the work tree, and then use 'git add/rm <file>' as\nappropriate to mark resolution and make a commit, or use 'git commit -a'.\n\nPull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD\nexists instead of waiting for merge to complain.\n\nThe behavior of merge and the test-case are slightly modified to reflect\nthe usual flow: start with conflicts, fix them, and afterwards get rid of\nMERGE_HEAD, with different error messages at each stage.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nAhem. *This* one passes the test-suite :-\\. Sorry for not having ran it earlier.\n\nSince the test for git-merge is changed a bit, I added the last\nparagraph of the commit message.\n\n Documentation/config.txt      |    4 ++++\n advice.c                      |   16 ++++++++++++++++\n advice.h                      |    5 +++++\n builtin-commit.c              |   14 ++++++++++++--\n builtin-merge.c               |   19 ++++++++++++++-----\n builtin-revert.c              |   15 ++++++++++++++-\n git-pull.sh                   |   25 +++++++++++++++++++++++--\n t/t3030-merge-recursive.sh    |    6 ++++--\n t/t3501-revert-cherry-pick.sh |    2 +-\n 9 files changed, 93 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 304eabb..8761411 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -130,6 +130,10 @@ advice.*::\n \t\tAdvice shown when linkgit:git-merge[1] refuses to\n \t\tmerge to avoid overwritting local changes.\n \t\tDefault: true.\n+\tresolveConflict::\n+\t\tAdvices shown by various commands when conflicts\n+\t\tprevent the operation from being performed. \n+\t\tDefault: true.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex cb666ac..3309521 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -3,6 +3,7 @@\n int advice_push_nonfastforward = 1;\n int advice_status_hints = 1;\n int advice_commit_before_merge = 1;\n+int advice_resolve_conflict = 1;\n \n static struct {\n \tconst char *name;\n@@ -11,6 +12,7 @@ static struct {\n \t{ \"pushnonfastforward\", &advice_push_nonfastforward },\n \t{ \"statushints\", &advice_status_hints },\n \t{ \"commitbeforemerge\", &advice_commit_before_merge },\n+\t{ \"resolveconflict\", &advice_resolve_conflict },\n };\n \n int git_default_advice_config(const char *var, const char *value)\n@@ -27,3 +29,17 @@ int git_default_advice_config(const char *var, const char *value)\n \n \treturn 0;\n }\n+\n+void NORETURN die_resolve_conflict(const char *me)\n+{\n+\tif (advice_resolve_conflict)\n+\t\t/*\n+\t\t * Message used both when 'git commit' fails and when\n+\t\t * other commands doing a merge do.\n+\t\t */\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n+\t\t    \"Please, fix them up in the work tree, and then use 'git add/rm <file>' as\\n\"\n+\t\t    \"appropriate to mark resolution and make a commit, or use 'git commit -a'.\", me);\n+\telse\n+\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n+}\ndiff --git a/advice.h b/advice.h\nindex 3de5000..acd5fdd 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -1,10 +1,15 @@\n #ifndef ADVICE_H\n #define ADVICE_H\n \n+#include \"git-compat-util.h\"\n+\n extern int advice_push_nonfastforward;\n extern int advice_status_hints;\n extern int advice_commit_before_merge;\n+extern int advice_resolve_conflict;\n \n int git_default_advice_config(const char *var, const char *value);\n \n+extern void NORETURN die_resolve_conflict(const char *me);\n+\n #endif /* ADVICE_H */\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 592b103..c56dca0 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -244,6 +244,16 @@ static void create_base_index(void)\n \t\texit(128); /* We've already reported the error, finish dying */\n }\n \n+static void refresh_cache_or_die(int refresh_flags)\n+{\n+\t/*\n+\t * refresh_flags contains REFRESH_QUIET, so the only errors\n+\t * are for unmerged entries.\n+\t */\n+\tif (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))\n+\t\tdie_resolve_conflict(\"commit\");\n+}\n+\n static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)\n {\n \tint fd;\n@@ -283,7 +293,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (all || (also && pathspec && *pathspec)) {\n \t\tint fd = hold_locked_index(&index_lock, 1);\n \t\tadd_files_to_cache(also ? prefix : NULL, pathspec, 0);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    close_lock_file(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\n@@ -302,7 +312,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t */\n \tif (!pathspec || !*pathspec) {\n \t\tfd = hold_locked_index(&index_lock, 1);\n-\t\trefresh_cache(refresh_flags);\n+\t\trefresh_cache_or_die(refresh_flags);\n \t\tif (write_cache(fd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&index_lock))\n \t\t\tdie(\"unable to write new_index file\");\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex f1c84d7..79a35c3 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -847,11 +847,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n \tstruct commit_list **remotes = &remoteheads;\n \n-\tif (file_exists(git_path(\"MERGE_HEAD\")))\n-\t\tdie(\"You have not concluded your merge. (MERGE_HEAD exists)\");\n-\tif (read_cache_unmerged())\n-\t\tdie(\"You are in the middle of a conflicted merge.\"\n-\t\t\t\t\" (index unmerged)\");\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(\"merge\");\n+\t}\n+\tif (file_exists(git_path(\"MERGE_HEAD\"))) {\n+\t\t/*\n+\t\t * There is no unmerged entry, don't advise 'git\n+\t\t * add/rm <file>', just 'git commit'.\n+\t\t */\n+\t\tif (advice_resolve_conflict)\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\\n\"\n+\t\t\t    \"Please, commit your changes before you can merge.\");\n+\t\telse\n+\t\t\tdie(\"You have not concluded your merge (MERGE_HEAD exists).\");\n+\t}\n \n \t/*\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 151aa6a..d14dde3 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)\n \treturn tree;\n }\n \n+static NORETURN void die_dirty_index(const char *me)\n+{\n+\tif (read_cache_unmerged()) {\n+\t\tdie_resolve_conflict(me);\n+\t} else {\n+\t\tif (advice_commit_before_merge)\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\"\n+\t\t\t    \"Please, commit your changes or stash them to proceed.\", me);\n+\t\telse\n+\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\", me);\n+\t}\n+}\n+\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\tif (get_sha1(\"HEAD\", head))\n \t\t\tdie (\"You do not have a valid HEAD\");\n \t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie (\"Dirty index: cannot %s\", me);\n+\t\t\tdie_dirty_index(me);\n \t}\n \tdiscard_cache();\n \ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9e69ada..54ce0af 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -13,8 +13,29 @@ set_reflog_action \"pull $*\"\n require_work_tree\n cd_to_toplevel\n \n-test -z \"$(git ls-files -u)\" ||\n-\tdie \"You are in the middle of a conflicted merge.\"\n+\n+die_conflict () {\n+    git diff-index --cached --name-status -r --ignore-submodules HEAD --\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"Pull is not possible because you have unmerged files.\n+Please, fix them up in the work tree, and then use 'git add/rm <file>'\n+as appropriate to mark resolution, or use 'git commit -a'.\"\n+    else\n+\tdie \"Pull is not possible because you have unmerged files.\"\n+    fi\n+}\n+\n+die_merge () {\n+    if [ $(git config --bool --get advice.resolveConflict || echo true) = \"true\" ]; then\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\n+Please, commit your changes before you can merge.\"\n+    else\n+\tdie \"You have not concluded your merge (MERGE_HEAD exists).\"\n+    fi\n+}\n+\n+test -z \"$(git ls-files -u)\" || die_conflict\n+test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity=\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 9b3fa2b..9929f82 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -276,11 +276,13 @@ test_expect_success 'fail if the index has unresolved entries' '\n \n \ttest_must_fail git merge \"$c5\" &&\n \ttest_must_fail git merge \"$c5\" 2> out &&\n+\tgrep \"not possible because you have unmerged files\" out &&\n+\tgit add -u &&\n+\ttest_must_fail git merge \"$c5\" 2> out &&\n \tgrep \"You have not concluded your merge\" out &&\n \trm -f .git/MERGE_HEAD &&\n \ttest_must_fail git merge \"$c5\" 2> out &&\n-\tgrep \"You are in the middle of a conflicted merge\" out\n-\n+\tgrep \"Your local changes to .* would be overwritten by merge.\" out\n '\n \n test_expect_success 'merge-recursive remove conflict' '\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex bb4cf00..7f85815 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -66,7 +66,7 @@ test_expect_success 'revert forbidden on dirty working tree' '\n \techo content >extra_file &&\n \tgit add extra_file &&\n \ttest_must_fail git revert HEAD 2>errors &&\n-\tgrep \"Dirty index\" errors\n+\tgrep \"Your local changes would be overwritten by \" errors\n \n '\n \n-- \n1.6.6.198.g3c5474\n"},{"id":"131434","messageId":"7v8wc2sdad.fsf@alter.siamese.dyndns.org","threadId":"22103","inReplyTo":"1262887810-19415-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v5] Be more user-friendly when refusing to do something because of conflict.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-13T06:56:58Z","receivedAt":"2010-01-13T06:56:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; will queue.\n"}]}