{"thread":{"id":"24930","subject":"[PATCH] Make sure show_all_errors when using porcelain error messages","startedAt":"2010-09-01T06:04:14Z","lastAt":"2010-09-03T15:25:33Z","messageCount":16,"participants":["Matthieu Moy","Junio C Hamano","Jim Meyering"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"149531","messageId":"vpqeide0zkx.fsf@bauges.imag.fr","threadId":"24930","inReplyTo":"20100901001729.GG6747@burratino","subject":"Re: [PATCH for next] insert missing newline in a diagnostic","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-01T06:04:14Z","receivedAt":"2010-09-01T06:04:14Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jonathan Nieder writes:\n\n> Jim Meyering wrote:\n>\n>> When merging, I would get a message like this:\n>> \n>> error: The following untracked working tree files would be overwritten by merge:\n>> FILE_NAMEPlease move or remove them before you can merge.\n>> \n>> This change inserts the newline after FILE_NAME.\n>\n> I fear it is more complicated. With your patch, in some situations\n> (e.g., when running t7609-merge-co-error-msgs.sh) I get a leading tab\n> and extra newline:\n>\n> error: The following untrack...\n> \tFILE_NAME\n>\n> Please move or remove them before you can merge.\n>\n> In unpack-trees, display_error_msgs() prints the version with a tab\n> but you are getting the message from add_rejected_path which suggests\n> to me that o->show_all_errors is unset.\n\nThat sounds like an explanation. The patch series did two things:\noverride plumbing error messages, and introducing show_all_errors. The\ntwo are meant to work together, and clearly, if you set one without\nthe other, you get surprising results. A fix would be to make sure\nthat we set both in the same places.\n\nPatch follows, untested. Jim, can you tell us whether it fixes the\nproblem? If not, can your give us a reproduction script (preferably as\na patch to t7609-merge-co-error-msgs.sh)?\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"149530","messageId":"1283321153-24745-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"vpqeide0zkx.fsf@bauges.imag.fr","subject":"[PATCH] Make sure show_all_errors when using porcelain error messages","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-01T06:05:53Z","receivedAt":"2010-09-01T06:05:53Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"opts.show_all_errors controls the number of errors that can be displayed,\nand set_porcelain_error_msgs gives the format. But the formatting would\nbe incorrect if set_porcelain_error_msgs is called without setting\nopts.show_all_errors.\n\nThere was a missing opts.show_all_errors in merge-recursive.c, and a case\nwhere it was not obvious enough in merge.c.\n\nSigned-off-by: Matthieu Moy \n---\nIf this is the issue, we should probably change\nset_porcelain_error_msgs to set show_all_errors itself (but that\nrequires changing the argument type)\n\n builtin/checkout.c | 2 +-\n merge-recursive.c | 1 +\n 2 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 7250e5c..8a7f994 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -376,6 +376,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.src_index = &the_index;\n \t\ttopts.dst_index = &the_index;\n \n+\t\ttopts.show_all_errors = 1;\n \t\tset_porcelain_error_msgs(topts.msgs, \"checkout\");\n \n \t\trefresh_cache(REFRESH_QUIET);\n@@ -395,7 +396,6 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n \t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n \t\ttopts.dir->exclude_per_dir = \".gitignore\";\n-\t\ttopts.show_all_errors = 1;\n \t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t old->commit->object.sha1 :\n \t\t\t\t\t (unsigned char *)EMPTY_TREE_SHA1_BIN);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 638076e..c7fc7a7 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -180,6 +180,7 @@ static int git_merge_trees(int index_only,\n \topts.fn = threeway_merge;\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n+\topts.show_all_errors = 1;\n \tset_porcelain_error_msgs(opts.msgs, \"merge\");\n \n \tinit_tree_desc_from_tree(t+0, common);\n-- \n1.7.2.2.175.ga619d.dirty\n"},{"id":"149563","messageId":"7vsk1t7bn2.fsf@alter.siamese.dyndns.org","threadId":"24930","inReplyTo":"1283321153-24745-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] Make sure show_all_errors when using porcelain error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-01T14:59:29Z","receivedAt":"2010-09-01T14:59:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> opts.show_all_errors controls the number of errors that can be displayed,\n> and set_porcelain_error_msgs gives the format. But the formatting would\n> be incorrect if set_porcelain_error_msgs is called without setting\n> opts.show_all_errors.\n\nThat makes it sound like a design bug of set_porcelain_error_msgs(), in\nthat the caller _cannot_ choose to stop at the first error if it wants to\nuse friendlier message than the plumbing one.\n\nBut if that is (and I do not think it is such a bad thing if we decide\nthat Porcelains have no option of stopping at the first error) then we\nshould set opts.show_all_errors in the set_porcelain_error_msgs()\nfunction, and probably should rename the function to something saner.\n\nThe function is not about setting the error messages anymore (and it\nsounds like it never has been since the current behaviour was introduced)\nbut is about declaring that we are a Porcelain and won't stop at the first\nerror. We might end up introducing different behaviours later in this\nfunction.\n\nHow about making it\n\n\tint setup_unpack_trees_porcelain(struct unpack_trees_options *opt,\n\t\t\t\t\t\tconst char *cmd);\n\nand possibly moving it from merge-recursive.c to unpack-trees.c?\n"},{"id":"149577","messageId":"vpq39ttxumz.fsf@bauges.imag.fr","threadId":"24930","inReplyTo":"7vsk1t7bn2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Make sure show_all_errors when using porcelain error messages","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-01T17:04:36Z","receivedAt":"2010-09-01T17:04:36Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> opts.show_all_errors controls the number of errors that can be displayed,\n>> and set_porcelain_error_msgs gives the format. But the formatting would\n>> be incorrect if set_porcelain_error_msgs is called without setting\n>> opts.show_all_errors.\n>\n> That makes it sound like a design bug of set_porcelain_error_msgs(), in\n> that the caller _cannot_ choose to stop at the first error if it wants to\n> use friendlier message than the plumbing one.\n\nYou're right that the caller cannot stop at the first error and get\nfriendly message, but I don't think this is a bug. As a user, I don't\nsee any reason not to want the complete list. We kept the\n\"show_all_errors = false\" behavior to allow the slight performance\nimprovement of stopping at the first error, which may (but I'm not\neven sure) benefit to scripts.\n\n> But if that is (and I do not think it is such a bad thing if we decide\n> that Porcelains have no option of stopping at the first error) then we\n> should set opts.show_all_errors in the set_porcelain_error_msgs()\n> function, and probably should rename the function to something\n> saner.\n\nYes (actually, I send a quick patch to make sure this was the problem\nof the OP, but the intention was to have a better one when I have\ntime. I forgot to add RFC in the title).\n\n> How about making it\n>\n> \tint setup_unpack_trees_porcelain(struct unpack_trees_options *opt,\n> \t\t\t\t\t\tconst char *cmd);\n>\n> and possibly moving it from merge-recursive.c to unpack-trees.c?\n\nSounds good, yes. Will do (probably tomorrow).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"149581","messageId":"7vfwxt5oyu.fsf@alter.siamese.dyndns.org","threadId":"24930","inReplyTo":"vpq39ttxumz.fsf@bauges.imag.fr","subject":"Re: [PATCH] Make sure show_all_errors when using porcelain error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-01T17:54:33Z","receivedAt":"2010-09-01T17:54:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> Matthieu Moy writes:\n>>\n>>> opts.show_all_errors controls the number of errors that can be displayed,\n>>> and set_porcelain_error_msgs gives the format. But the formatting would\n>>> be incorrect if set_porcelain_error_msgs is called without setting\n>>> opts.show_all_errors.\n>>\n>> That makes it sound like a design bug of set_porcelain_error_msgs(), in\n>> that the caller _cannot_ choose to stop at the first error if it wants to\n>> use friendlier message than the plumbing one.\n>\n> You're right that the caller cannot stop at the first error and get\n> friendly message, but I don't think this is a bug.\n\nI am not saying it is a bug. The description makes it sound like it is a\nbug and I was wondering if it can be worded better not to do so.\n"},{"id":"149624","messageId":"87lj7kimjy.fsf@meyering.net","threadId":"24930","inReplyTo":"vpqeide0zkx.fsf@bauges.imag.fr","subject":"Re: [PATCH for next] insert missing newline in a diagnostic","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2010-09-02T08:20:33Z","receivedAt":"2010-09-02T08:20:33Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Matthieu Moy wrote:\n> Jonathan Nieder writes:\n>\n>> Jim Meyering wrote:\n>>\n>>> When merging, I would get a message like this:\n>>>\n>>> error: The following untracked working tree files would be overwritten by merge:\n>>> FILE_NAMEPlease move or remove them before you can merge.\n>>>\n>>> This change inserts the newline after FILE_NAME.\n>>\n>> I fear it is more complicated. With your patch, in some situations\n>> (e.g., when running t7609-merge-co-error-msgs.sh) I get a leading tab\n>> and extra newline:\n>>\n>> error: The following untrack...\n>> \tFILE_NAME\n>>\n>> Please move or remove them before you can merge.\n>>\n>> In unpack-trees, display_error_msgs() prints the version with a tab\n>> but you are getting the message from add_rejected_path which suggests\n>> to me that o->show_all_errors is unset.\n>\n> That sounds like an explanation. The patch series did two things:\n> override plumbing error messages, and introducing show_all_errors. The\n> two are meant to work together, and clearly, if you set one without\n> the other, you get surprising results. A fix would be to make sure\n> that we set both in the same places.\n\nThanks to both of you.\n\n> Patch follows, untested. Jim, can you tell us whether it fixes the\n> problem? If not, can your give us a reproduction script (preferably as\n> a patch to t7609-merge-co-error-msgs.sh)?\n\nMatthieu, should your patch have gone to the mailing list?\nI do not see it there.\n"},{"id":"149626","messageId":"vpq8w3klenl.fsf@bauges.imag.fr","threadId":"24930","inReplyTo":"87lj7kimjy.fsf@meyering.net","subject":"Re: [PATCH for next] insert missing newline in a diagnostic","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-02T08:42:54Z","receivedAt":"2010-09-02T08:42:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jim Meyering writes:\n\n>> Patch follows, untested. Jim, can you tell us whether it fixes the\n>> problem? If not, can your give us a reproduction script (preferably as\n>> a patch to t7609-merge-co-error-msgs.sh)?\n>\n> Matthieu, should your patch have gone to the mailing list?\n> I do not see it there.\n\nhttp://article.gmane.org/gmane.comp.version-control.git/155016\n\nJunio replied to it, so it must have gone through ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"149637","messageId":"1283428655-12680-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"vpq39ttxumz.fsf@bauges.imag.fr","subject":"[PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-02T11:57:32Z","receivedAt":"2010-09-02T11:57:32Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"So, after my toy untested patch, here's a cleaner version. I splitted\nit into really small patches, to avoid modifying code while I move it,\nbut Junio, feel free to squash 1/3 and 2/3 if you prefer.\n\nThis fixes a regression, so this should be in the next release if the\npatch is OK.\n\nJim: can you confirm if it fixes your problem?\n\nMatthieu Moy (3):\n Move set_porcelain_error_msgs to unpack-trees.c and rename it\n setup_unpack_trees_porcelain: take the whole options struct as\n parameter\n Move \"show_all_errors = 1\" to setup_unpack_trees_porcelain()\n\n builtin/checkout.c | 3 +-\n builtin/merge.c | 3 +-\n merge-recursive.c | 46 +-----------------------------------------\n merge-recursive.h | 6 -----\n unpack-trees.c | 56 +++++++++++++++++++++++++++++++++++++++++++++------\n unpack-trees.h | 7 ++++++\n 6 files changed, 59 insertions(+), 62 deletions(-)\n\n-- \n1.7.2.2.175.ga619d.dirty\n"},{"id":"149639","messageId":"1283428655-12680-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"vpq39ttxumz.fsf@bauges.imag.fr","subject":"[PATCH 1/3] Move set_porcelain_error_msgs to unpack-trees.c and rename it","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-02T11:57:33Z","receivedAt":"2010-09-02T11:57:33Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The function is currently dealing only with error messages, but the\nintent of calling it is really to notify the unpack-tree mechanics that\nit is running in porcelain mode.\n\nSigned-off-by: Matthieu Moy \n---\n builtin/checkout.c | 2 +-\n builtin/merge.c | 2 +-\n merge-recursive.c | 46 +---------------------------------------------\n merge-recursive.h | 6 ------\n unpack-trees.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-\n unpack-trees.h | 6 ++++++\n 6 files changed, 54 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 7250e5c..e5c0ef0 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -376,7 +376,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.src_index = &the_index;\n \t\ttopts.dst_index = &the_index;\n \n-\t\tset_porcelain_error_msgs(topts.msgs, \"checkout\");\n+\t\tsetup_unpack_trees_porcelain(topts.msgs, \"checkout\");\n \n \t\trefresh_cache(REFRESH_QUIET);\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 47e705b..da52b10 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -706,7 +706,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \topts.merge = 1;\n \topts.fn = twoway_merge;\n \topts.show_all_errors = 1;\n-\tset_porcelain_error_msgs(opts.msgs, \"merge\");\n+\tsetup_unpack_trees_porcelain(opts.msgs, \"merge\");\n \n \ttrees[nr_trees] = parse_tree_indirect(head);\n \tif (!trees[nr_trees++])\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex df90be4..61e237b 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -180,7 +180,7 @@ static int git_merge_trees(int index_only,\n \topts.fn = threeway_merge;\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n-\tset_porcelain_error_msgs(opts.msgs, \"merge\");\n+\tsetup_unpack_trees_porcelain(opts.msgs, \"merge\");\n \n \tinit_tree_desc_from_tree(t+0, common);\n \tinit_tree_desc_from_tree(t+1, head);\n@@ -1238,50 +1238,6 @@ static int process_df_entry(struct merge_options *o,\n \treturn clean_merge;\n }\n \n-void set_porcelain_error_msgs(const char **msgs, const char *cmd)\n-{\n-\tconst char *msg;\n-\tchar *tmp;\n-\tconst char *cmd2 = strcmp(cmd, \"checkout\") ? cmd : \"switch branches\";\n-\tif (advice_commit_before_merge)\n-\t\tmsg = \"Your local changes to the following files would be overwritten by %s:\\n%%s\"\n-\t\t\t\"Please, commit your changes or stash them before you can %s.\";\n-\telse\n-\t\tmsg = \"Your local changes to the following files would be overwritten by %s:\\n%%s\";\n-\ttmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2);\n-\tsprintf(tmp, msg, cmd, cmd2);\n-\tmsgs[ERROR_WOULD_OVERWRITE] = tmp;\n-\tmsgs[ERROR_NOT_UPTODATE_FILE] = tmp;\n-\n-\tmsgs[ERROR_NOT_UPTODATE_DIR] =\n-\t\t\"Updating the following directories would lose untracked files in it:\\n%s\";\n-\n-\tif (advice_commit_before_merge)\n-\t\tmsg = \"The following untracked working tree files would be %s by %s:\\n%%s\"\n-\t\t\t\"Please move or remove them before you can %s.\";\n-\telse\n-\t\tmsg = \"The following untracked working tree files would be %s by %s:\\n%%s\";\n-\ttmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(\"removed\") + strlen(cmd2) - 4);\n-\tsprintf(tmp, msg, \"removed\", cmd, cmd2);\n-\tmsgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp;\n-\ttmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(\"overwritten\") + strlen(cmd2) - 4);\n-\tsprintf(tmp, msg, \"overwritten\", cmd, cmd2);\n-\tmsgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp;\n-\n-\t/*\n-\t * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we\n-\t * cannot easily display it as a list.\n-\t */\n-\tmsgs[ERROR_BIND_OVERLAP] = \"Entry '%s' overlaps with '%s'. Cannot bind.\";\n-\n-\tmsgs[ERROR_SPARSE_NOT_UPTODATE_FILE] =\n-\t\t\"Cannot update sparse checkout: the following entries are not up-to-date:\\n%s\";\n-\tmsgs[ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN] =\n-\t\t\"The following Working tree files would be overwritten by sparse checkout update:\\n%s\";\n-\tmsgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =\n-\t\t\"The following Working tree files would be removed by sparse checkout update:\\n%s\";\n-}\n-\n int merge_trees(struct merge_options *o,\n \t\tstruct tree *head,\n \t\tstruct tree *merge,\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 08f9850..f79917c 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -23,12 +23,6 @@ struct merge_options {\n \tstruct string_list current_directory_set;\n };\n \n-/*\n- * Sets the list of user-friendly error messages to be used by the\n- * command \"cmd\" (either merge or checkout)\n- */\n-void set_porcelain_error_msgs(const char **msgs, const char *cmd);\n-\n /* merge_trees() but with recursive ancestor consolidation */\n int merge_recursive(struct merge_options *o,\n \t\t struct commit *h1,\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 3c7a7c9..4520aa0 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -14,7 +14,7 @@\n * read-tree. Non-scripted Porcelain is not required to use these messages\n * and in fact are encouraged to reword them to better suit their particular\n * situation better. See how \"git checkout\" and \"git merge\" replaces\n- * them using set_porcelain_error_msgs(), for example.\n+ * them using setup_unpack_trees_porcelain(), for example.\n */\n const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {\n \t/* ERROR_WOULD_OVERWRITE */\n@@ -50,6 +50,50 @@ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {\n \t ? ((o)->msgs[(type)]) \\\n \t : (unpack_plumbing_errors[(type)]) )\n \n+void setup_unpack_trees_porcelain(const char **msgs, const char *cmd)\n+{\n+\tconst char *msg;\n+\tchar *tmp;\n+\tconst char *cmd2 = strcmp(cmd, \"checkout\") ? cmd : \"switch branches\";\n+\tif (advice_commit_before_merge)\n+\t\tmsg = \"Your local changes to the following files would be overwritten by %s:\\n%%s\"\n+\t\t\t\"Please, commit your changes or stash them before you can %s.\";\n+\telse\n+\t\tmsg = \"Your local changes to the following files would be overwritten by %s:\\n%%s\";\n+\ttmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2);\n+\tsprintf(tmp, msg, cmd, cmd2);\n+\tmsgs[ERROR_WOULD_OVERWRITE] = tmp;\n+\tmsgs[ERROR_NOT_UPTODATE_FILE] = tmp;\n+\n+\tmsgs[ERROR_NOT_UPTODATE_DIR] =\n+\t\t\"Updating the following directories would lose untracked files in it:\\n%s\";\n+\n+\tif (advice_commit_before_merge)\n+\t\tmsg = \"The following untracked working tree files would be %s by %s:\\n%%s\"\n+\t\t\t\"Please move or remove them before you can %s.\";\n+\telse\n+\t\tmsg = \"The following untracked working tree files would be %s by %s:\\n%%s\";\n+\ttmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(\"removed\") + strlen(cmd2) - 4);\n+\tsprintf(tmp, msg, \"removed\", cmd, cmd2);\n+\tmsgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp;\n+\ttmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(\"overwritten\") + strlen(cmd2) - 4);\n+\tsprintf(tmp, msg, \"overwritten\", cmd, cmd2);\n+\tmsgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp;\n+\n+\t/*\n+\t * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we\n+\t * cannot easily display it as a list.\n+\t */\n+\tmsgs[ERROR_BIND_OVERLAP] = \"Entry '%s' overlaps with '%s'. Cannot bind.\";\n+\n+\tmsgs[ERROR_SPARSE_NOT_UPTODATE_FILE] =\n+\t\t\"Cannot update sparse checkout: the following entries are not up-to-date:\\n%s\";\n+\tmsgs[ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN] =\n+\t\t\"The following Working tree files would be overwritten by sparse checkout update:\\n%s\";\n+\tmsgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =\n+\t\t\"The following Working tree files would be removed by sparse checkout update:\\n%s\";\n+}\n+\n static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,\n \tunsigned int set, unsigned int clear)\n {\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 6e049b0..d62bba9 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -22,6 +22,12 @@ enum unpack_trees_error_types {\n \tNB_UNPACK_TREES_ERROR_TYPES\n };\n \n+/*\n+ * Sets the list of user-friendly error messages to be used by the\n+ * command \"cmd\" (either merge or checkout)\n+ */\n+void setup_unpack_trees_porcelain(const char **msgs, const char *cmd);\n+\n struct rejected_paths_list {\n \tchar *path;\n \tstruct rejected_paths_list *next;\n-- \n1.7.2.2.175.ga619d.dirty\n"},{"id":"149636","messageId":"1283428655-12680-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"vpq39ttxumz.fsf@bauges.imag.fr","subject":"[PATCH 2/3] setup_unpack_trees_porcelain: take the whole options struct as parameter","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-02T11:57:34Z","receivedAt":"2010-09-02T11:57:34Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This is a preparation patch to let setup_unpack_trees_porcelain set\nshow_all_errors itself.\n\nSigned-off-by: Matthieu Moy \n---\n builtin/checkout.c | 2 +-\n builtin/merge.c | 2 +-\n merge-recursive.c | 2 +-\n unpack-trees.c | 4 +++-\n unpack-trees.h | 3 ++-\n 5 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e5c0ef0..b26dfd0 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -376,7 +376,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.src_index = &the_index;\n \t\ttopts.dst_index = &the_index;\n \n-\t\tsetup_unpack_trees_porcelain(topts.msgs, \"checkout\");\n+\t\tsetup_unpack_trees_porcelain(&topts, \"checkout\");\n \n \t\trefresh_cache(REFRESH_QUIET);\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex da52b10..389e79c 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -706,7 +706,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \topts.merge = 1;\n \topts.fn = twoway_merge;\n \topts.show_all_errors = 1;\n-\tsetup_unpack_trees_porcelain(opts.msgs, \"merge\");\n+\tsetup_unpack_trees_porcelain(&opts, \"merge\");\n \n \ttrees[nr_trees] = parse_tree_indirect(head);\n \tif (!trees[nr_trees++])\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 61e237b..ebe6700 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -180,7 +180,7 @@ static int git_merge_trees(int index_only,\n \topts.fn = threeway_merge;\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n-\tsetup_unpack_trees_porcelain(opts.msgs, \"merge\");\n+\tsetup_unpack_trees_porcelain(&opts, \"merge\");\n \n \tinit_tree_desc_from_tree(t+0, common);\n \tinit_tree_desc_from_tree(t+1, head);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 4520aa0..17501d3 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -50,8 +50,10 @@ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {\n \t ? ((o)->msgs[(type)]) \\\n \t : (unpack_plumbing_errors[(type)]) )\n \n-void setup_unpack_trees_porcelain(const char **msgs, const char *cmd)\n+void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n+\t\t\t\t const char *cmd)\n {\n+\tconst char **msgs = opts->msgs;\n \tconst char *msg;\n \tchar *tmp;\n \tconst char *cmd2 = strcmp(cmd, \"checkout\") ? cmd : \"switch branches\";\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex d62bba9..fad680d 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -26,7 +26,8 @@ enum unpack_trees_error_types {\n * Sets the list of user-friendly error messages to be used by the\n * command \"cmd\" (either merge or checkout)\n */\n-void setup_unpack_trees_porcelain(const char **msgs, const char *cmd);\n+void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n+\t\t\t\t const char *cmd);\n \n struct rejected_paths_list {\n \tchar *path;\n-- \n1.7.2.2.175.ga619d.dirty\n"},{"id":"149638","messageId":"1283428655-12680-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"vpq39ttxumz.fsf@bauges.imag.fr","subject":"[PATCH 3/3] Move \"show_all_errors = 1\" to setup_unpack_trees_porcelain()","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-02T11:57:35Z","receivedAt":"2010-09-02T11:57:35Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Not only this makes the code clearer since setting up the porcelain error\nmessage is meant to work with show_all_errors, but this fixes a call to\nsetup_unpack_trees_porcelain() in git_merge_trees() which did not set\nshow_all_errors.\n\nadd_rejected_path() used to double-check whether it was running in\nplumbing mode. This check was inefficient since it was setting\nshow_all_errors too late for traverse_trees() to see it, and is made\nuseless by this patch. Remove it.\n\nSigned-off-by: Matthieu Moy \n---\n builtin/checkout.c | 1 -\n builtin/merge.c | 1 -\n unpack-trees.c | 8 ++------\n unpack-trees.h | 2 +-\n 4 files changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b26dfd0..1532669 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -395,7 +395,6 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n \t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n \t\ttopts.dir->exclude_per_dir = \".gitignore\";\n-\t\ttopts.show_all_errors = 1;\n \t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t old->commit->object.sha1 :\n \t\t\t\t\t (unsigned char *)EMPTY_TREE_SHA1_BIN);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 389e79c..bfd3d32 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -705,7 +705,6 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \topts.verbose_update = 1;\n \topts.merge = 1;\n \topts.fn = twoway_merge;\n-\topts.show_all_errors = 1;\n \tsetup_unpack_trees_porcelain(&opts, \"merge\");\n \n \ttrees[nr_trees] = parse_tree_indirect(head);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 17501d3..803445a 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -94,6 +94,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n \t\t\"The following Working tree files would be overwritten by sparse checkout update:\\n%s\";\n \tmsgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =\n \t\t\"The following Working tree files would be removed by sparse checkout update:\\n%s\";\n+\n+\topts->show_all_errors = 1;\n }\n \n static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,\n@@ -123,12 +125,6 @@ static int add_rejected_path(struct unpack_trees_options *o,\n \t\t\t const char *path)\n {\n \tstruct rejected_paths_list *newentry;\n-\tint porcelain = o && (o)->msgs[e];\n-\t/*\n-\t * simply display the given error message if in plumbing mode\n-\t */\n-\tif (!porcelain)\n-\t\to->show_all_errors = 0;\n \tif (!o->show_all_errors)\n \t\treturn error(ERRORMSG(o, e), path);\n \ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex fad680d..7c0187d 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -24,7 +24,7 @@ enum unpack_trees_error_types {\n \n /*\n * Sets the list of user-friendly error messages to be used by the\n- * command \"cmd\" (either merge or checkout)\n+ * command \"cmd\" (either merge or checkout), and show_all_errors to 1.\n */\n void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n \t\t\t\t const char *cmd);\n-- \n1.7.2.2.175.ga619d.dirty\n"},{"id":"149652","messageId":"7v8w3k2ldz.fsf@alter.siamese.dyndns.org","threadId":"24930","inReplyTo":"1283428655-12680-4-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 3/3] Move \"show_all_errors = 1\" to setup_unpack_trees_porcelain()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-02T15:52:24Z","receivedAt":"2010-09-02T15:52:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Not only this makes the code clearer since setting up the porcelain error\n> message is meant to work with show_all_errors, but this fixes a call to\n> setup_unpack_trees_porcelain() in git_merge_trees() which did not set\n> show_all_errors.\n>\n> add_rejected_path() used to double-check whether it was running in\n> plumbing mode. This check was inefficient since it was setting\n> show_all_errors too late for traverse_trees() to see it, and is made\n> useless by this patch. Remove it.\n\nDo you mean inefficient or ineffective?\n\nThe code in this patch looks fine, though.\n\nThanks.\n"},{"id":"149654","messageId":"vpq1v9cgmfe.fsf@bauges.imag.fr","threadId":"24930","inReplyTo":"7v8w3k2ldz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] Move \"show_all_errors = 1\" to setup_unpack_trees_porcelain()","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-02T16:06:13Z","receivedAt":"2010-09-02T16:06:13Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> Not only this makes the code clearer since setting up the porcelain error\n>> message is meant to work with show_all_errors, but this fixes a call to\n>> setup_unpack_trees_porcelain() in git_merge_trees() which did not set\n>> show_all_errors.\n>>\n>> add_rejected_path() used to double-check whether it was running in\n>> plumbing mode. This check was inefficient since it was setting\n>> show_all_errors too late for traverse_trees() to see it, and is made\n>> useless by this patch. Remove it.\n>\n> Do you mean inefficient or ineffective?\n\nIneffective, yes.\n\n\nI should have listened better during my english courses ;-). Both\nwords translate to \"inefficace\" in French, and it's a common mistake\nfor french people.\n\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"149664","messageId":"1283443695-17004-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"vpq1v9cgmfe.fsf@bauges.imag.fr","subject":"[PATCH v2] Move \"show_all_errors = 1\" to setup_unpack_trees_porcelain()","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-02T16:08:15Z","receivedAt":"2010-09-02T16:08:15Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Not only this makes the code clearer since setting up the porcelain error\nmessage is meant to work with show_all_errors, but this fixes a call to\nsetup_unpack_trees_porcelain() in git_merge_trees() which did not set\nshow_all_errors.\n\nadd_rejected_path() used to double-check whether it was running in\nplumbing mode. This check was ineffective since it was setting\nshow_all_errors too late for traverse_trees() to see it, and is made\nuseless by this patch. Remove it.\n\nSigned-off-by: Matthieu Moy \n---\nJust s/inefficient/ineffective/ compared to v1\n\n builtin/checkout.c | 1 -\n builtin/merge.c | 1 -\n unpack-trees.c | 8 ++------\n unpack-trees.h | 2 +-\n 4 files changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b26dfd0..1532669 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -395,7 +395,6 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n \t\ttopts.dir->flags |= DIR_SHOW_IGNORED;\n \t\ttopts.dir->exclude_per_dir = \".gitignore\";\n-\t\ttopts.show_all_errors = 1;\n \t\ttree = parse_tree_indirect(old->commit ?\n \t\t\t\t\t old->commit->object.sha1 :\n \t\t\t\t\t (unsigned char *)EMPTY_TREE_SHA1_BIN);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 389e79c..bfd3d32 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -705,7 +705,6 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote\n \topts.verbose_update = 1;\n \topts.merge = 1;\n \topts.fn = twoway_merge;\n-\topts.show_all_errors = 1;\n \tsetup_unpack_trees_porcelain(&opts, \"merge\");\n \n \ttrees[nr_trees] = parse_tree_indirect(head);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 17501d3..803445a 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -94,6 +94,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n \t\t\"The following Working tree files would be overwritten by sparse checkout update:\\n%s\";\n \tmsgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =\n \t\t\"The following Working tree files would be removed by sparse checkout update:\\n%s\";\n+\n+\topts->show_all_errors = 1;\n }\n \n static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,\n@@ -123,12 +125,6 @@ static int add_rejected_path(struct unpack_trees_options *o,\n \t\t\t const char *path)\n {\n \tstruct rejected_paths_list *newentry;\n-\tint porcelain = o && (o)->msgs[e];\n-\t/*\n-\t * simply display the given error message if in plumbing mode\n-\t */\n-\tif (!porcelain)\n-\t\to->show_all_errors = 0;\n \tif (!o->show_all_errors)\n \t\treturn error(ERRORMSG(o, e), path);\n \ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex fad680d..7c0187d 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -24,7 +24,7 @@ enum unpack_trees_error_types {\n \n /*\n * Sets the list of user-friendly error messages to be used by the\n- * command \"cmd\" (either merge or checkout)\n+ * command \"cmd\" (either merge or checkout), and show_all_errors to 1.\n */\n void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n \t\t\t\t const char *cmd);\n-- \n1.7.2.2.175.ga619d.dirty\n"},{"id":"149761","messageId":"8762ymewq5.fsf@meyering.net","threadId":"24930","inReplyTo":"1283428655-12680-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2010-09-03T14:18:58Z","receivedAt":"2010-09-03T14:18:58Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Matthieu Moy wrote:\n> So, after my toy untested patch, here's a cleaner version. I splitted\n> it into really small patches, to avoid modifying code while I move it,\n> but Junio, feel free to squash 1/3 and 2/3 if you prefer.\n>\n> This fixes a regression, so this should be in the next release if the\n> patch is OK.\n>\n> Jim: can you confirm if it fixes your problem?\n\nHi Matthieu,\nThanks for all of those patches.\nUnfortunately, I was unable to reproduce the state\nthat provoked my problem.\n"},{"id":"149762","messageId":"1283527533-16736-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24930","inReplyTo":"8762ymewq5.fsf@meyering.net","subject":"[PATCH] t7609-merge-co-error-msgs: test non-fast forward case too.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-09-03T15:25:33Z","receivedAt":"2010-09-03T15:25:33Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\nThis comes on top of the patch serie, and adds a test to make sure the\nbug doesn't come back.\n\nI did check manually that the git of git.git doesn't pass this (while\nthe one after the patch serie does).\n\n t/t7609-merge-co-error-msgs.sh | 12 ++++++++++--\n 1 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7609-merge-co-error-msgs.sh b/t/t7609-merge-co-error-msgs.sh\nindex 1a109b4..6b58299 100755\n--- a/t/t7609-merge-co-error-msgs.sh\n+++ b/t/t7609-merge-co-error-msgs.sh\n@@ -34,9 +34,17 @@ error: The following untracked working tree files would be overwritten by merge:\n Please move or remove them before you can merge.\n EOF\n \n-test_expect_success 'untracked files overwritten by merge' '\n+test_expect_success 'untracked files overwritten by merge (fast and non-fast forward)' '\n \ttest_must_fail git merge branch 2>out &&\n-\ttest_cmp out expect\n+\ttest_cmp out expect &&\n+\tgit commit --allow-empty -m empty &&\n+\t(\n+\t\tGIT_MERGE_VERBOSITY=0 &&\n+\t\texport GIT_MERGE_VERBOSITY &&\n+\t\ttest_must_fail git merge branch 2>out2\n+\t) &&\n+\ttest_cmp out2 expect &&\n+\tgit reset --hard HEAD^\t\n '\n \n cat >expect <<\\EOF\n-- \n1.7.2.2.284.g95cc.dirty\n"}]}