{"thread":{"id":"20408","subject":"Message from git reset: confusing?","startedAt":"2009-08-05T15:25:23Z","lastAt":"2009-08-23T11:45:18Z","messageCount":20,"participants":["Matthieu Moy","Junio C Hamano","Avery Pennarun","John Tapsell","Sverre Rabbelier","Reece Dunn"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"119622","messageId":"vpqab2e7064.fsf@bauges.imag.fr","threadId":"20408","inReplyTo":null,"subject":"Message from git reset: confusing?","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-05T15:25:23Z","receivedAt":"2009-08-05T15:25:23Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Hi,\n\nI was wondering what was the motivation for the output of \"git merge\":\n\n$ git reset file\nfile: locally modified\n$ \n\nI find this message misleading, since it gives me the feeling that\n\"git reset\" errored out because of the file being locally modified (I\nalso got the remark from a git beginner to whom I was showing the\ncommand: \"hey, why didn't it work??\").\n\nAnd indeed, I do not understand the motivation for showing this\nmessage. When I stage content (git add), Git tells me nothing, so why\nshould it do so when I unstage content? If I want to know the state of\nthe index after running reset, I'll run \"git status\" myself ...\n\nI'd suggest adding a -v|--verbose flag to git reset, and default to\nbeing quiet (the already existing -q flag would still be used, to\ndisable progress indicator). What do other people think?\n\n-- \nMatthieu\n"},{"id":"119635","messageId":"7v1vnqb2hc.fsf@alter.siamese.dyndns.org","threadId":"20408","inReplyTo":"vpqab2e7064.fsf@bauges.imag.fr","subject":"Re: Message from git reset: confusing?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-05T17:21:51Z","receivedAt":"2009-08-05T17:21:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> I was wondering what was the motivation for the output of \"git merge\":\n\nYou meant \"git reset\".\n\n> $ git reset file\n> file: locally modified\n> $ \n\nFirst a tangent.\n\nRemoving this output _when no <path> is given_ would greatly reduce the\nusability of the command.\n\nI often find myself working on something that consists of more than one\nsteps, and initially I decide that these would form, say, two commits A,\nand B.  I first start adding the changes that are relevant to A, and my\nindex gradually gets closer to what A should finally look like.\n\nBut then I realize that logically commit B should come before commit A,\nand it is time to \"git reset\" without any <path>s.  The output would let\nme review the changes (this includes changes pertaining to both A and B)\nto help me recall which files would contain changes that are relevant to\nB.\n\nI agree that \"git reset a-single-exact-filename\" could be much more\nsilent.  I would even say we do not even need -v in such a case.\n\nBut the thing is, that is a very narrow special case.  The parameter the\ncommand takes is not _a file_, but is a set of pathspecs, and I would\nimagine that when you are in a situation similar to what I just described\nin a larger project, you would appreciate the same reminder of modified\npaths when you run the command like this:\n\n    $ git reset include/ arch/x86/\n\nI wouldn't oppose to a patch that squelches the output when all pathspecs\ngiven from the command line _exactly_ name existing paths, but I tend to\nthink that it would be usability regression if you do not show any output\nin a case like the last example.\n"},{"id":"119641","messageId":"32541b130908051042x5308e8fte7b3ead6bf1f24ee@mail.gmail.com","threadId":"20408","inReplyTo":"7v1vnqb2hc.fsf@alter.siamese.dyndns.org","subject":"Re: Message from git reset: confusing?","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-08-05T17:42:53Z","receivedAt":"2009-08-05T17:42:53Z","isPatch":false,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Wed, Aug 5, 2009 at 5:21 PM, Junio C Hamano<gitster@pobox.com> wrote:\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>\n>> I was wondering what was the motivation for the output of \"git merge\":\n>\n> You meant \"git reset\".\n>\n>> $ git reset file\n>> file: locally modified\n>> $\n>\n> First a tangent.\n>\n> Removing this output _when no <path> is given_ would greatly reduce the\n> usability of the command.\n\nYes.  I think the problem is that the current output looks more like\nan error message than a status report.\n\nI would find it very pleasant if the output looked more like the\noutput of \"git checkout\" (no parameters) in the no-files-specified\ncase.\n\nEven if people don't know what \"M\" means on day 1 (although hopefully\nthey don't need \"git reset\" on day 1), at least it doesn't look like\nan error message.\n\nHave fun,\n\nAvery\n"},{"id":"119645","messageId":"43d8ce650908051107o5df3a780j7481bc364ae3062d@mail.gmail.com","threadId":"20408","inReplyTo":"32541b130908051042x5308e8fte7b3ead6bf1f24ee@mail.gmail.com","subject":"Re: Message from git reset: confusing?","fromName":"John Tapsell","fromEmail":"johnflux@gmail.com","sentAt":"2009-08-05T18:07:33Z","receivedAt":"2009-08-05T18:07:33Z","isPatch":false,"sender":{"key":"johnflux@gmail.com","avatar":"https://gravatar.com/avatar/25f70d4c0f96396b84a2e34bcd9bdc233462c7b4be29b5fdca8266fc53f30b0c?d=mp&s=160"},"body":"> Even if people don't know what \"M\" means on day 1 (although hopefully\n> they don't need \"git reset\" on day 1), at least it doesn't look like\n> an error message.\n\nActually for newbies, git reset is pretty much the first thing they\nlearn to try to undo the mess that they created.\n\nAlmost the very first thing a newbie to git does is something like:\n\n$ git checkout experimental_branch\n<modify file>\n$ git commit\n..\n(what do you mean I've committed to a detached head?  why is\neverything going wrong?  what's happening!!)\n\n$ git reset\n"},{"id":"119651","messageId":"fabb9a1e0908051125n3e125209n88a9fd86d6fa7534@mail.gmail.com","threadId":"20408","inReplyTo":"32541b130908051042x5308e8fte7b3ead6bf1f24ee@mail.gmail.com","subject":"Re: Message from git reset: confusing?","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-05T18:25:07Z","receivedAt":"2009-08-05T18:25:07Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Aug 5, 2009 at 10:42, Avery Pennarun<apenwarr@gmail.com> wrote:\n> Yes.  I think the problem is that the current output looks more like\n> an error message than a status report.\n\nDefinitely.\n\n> I would find it very pleasant if the output looked more like the\n> output of \"git checkout\" (no parameters) in the no-files-specified\n> case.\n\nPerhaps instead we could get away with simply adding a header like\n'git status' does? And perhaps change the wording some.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"119763","messageId":"vpqprb946sk.fsf@bauges.imag.fr","threadId":"20408","inReplyTo":"fabb9a1e0908051125n3e125209n88a9fd86d6fa7534@mail.gmail.com","subject":"Re: Message from git reset: confusing?","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-06T09:42:51Z","receivedAt":"2009-08-06T09:42:51Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> writes:\n\n> Heya,\n>\n> On Wed, Aug 5, 2009 at 10:42, Avery Pennarun<apenwarr@gmail.com> wrote:\n>> Yes.  I think the problem is that the current output looks more like\n>> an error message than a status report.\n>\n> Definitely.\n\nThis is my biggest issue, indeed. Actually, several things bother me\n(by decreasing annoyance):\n\n1) It looks like an error message, and user can think git reset\nfailed.\n\n2) It's inconsistant with the usual status display. I'd prefer an\noutput like \"git diff --name-only\" or \"git status\".\n\n3) It's verbose. Junio's reply addresses this point. I'm not really\nconvinced that verbosity by default is a good thing, but I don't think\nI can convince Junio either ;-), and I don't care that much.\n\nSo, let's address 1) and 2) only.\n\n>> I would find it very pleasant if the output looked more like the\n>> output of \"git checkout\" (no parameters) in the no-files-specified\n>> case.\n>\n> Perhaps instead we could get away with simply adding a header like\n> 'git status' does? And perhaps change the wording some.\n\nJust this patch would already solve 1) above mostly. At first, I\nwanted the message to say \"reset successfull\", but it's harder than it\nseems, since the output is printf-ed as the work is done, so one knows\nthat reset is successful only after displaying the whole thing:\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 5fa1789..6b16a00 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -108,6 +108,7 @@ static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n        if (read_cache() < 0)\n                return error(\"Could not read index\");\n \n+       printf(\"Unstaged changes after reset:\\n\");\n        result = refresh_cache(flags) ? 1 : 0;\n        if (write_cache(fd, active_cache, active_nr) ||\n                        commit_locked_index(index_lock))\n\nFor 2), something along the lines of:\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 4e3e272..3a99a2b 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1075,10 +1075,13 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n        int not_new = (flags & REFRESH_IGNORE_MISSING) != 0;\n        int ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;\n        unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n-       const char *needs_update_message;\n+       const char *needs_update_fmt;\n+       const char *needs_merge_fmt;\n \n-       needs_update_message = ((flags & REFRESH_SAY_CHANGED)\n-                               ? \"locally modified\" : \"needs update\");\n+       needs_update_fmt = ((flags & REFRESH_SAY_CHANGED)\n+                               ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+       needs_merge_fmt = ((flags & REFRESH_SAY_CHANGED)\n+                               ? \"U\\t%s\\n\" : \"%s: needs merge\\n\");\n        for (i = 0; i < istate->cache_nr; i++) {\n                struct cache_entry *ce, *new;\n                int cache_errno = 0;\n@@ -1094,7 +1097,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n                        i--;\n                        if (allow_unmerged)\n                                continue;\n-                       printf(\"%s: needs merge\\n\", ce->name);\n+                       printf(needs_merge_fmt, ce->name);\n                        has_errors = 1;\n                        continue;\n                }\n@@ -1117,7 +1120,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n                        }\n                        if (quiet)\n                                continue;\n-                       printf(\"%s: %s\\n\", ce->name, needs_update_message);\n+                       printf(needs_update_fmt, ce->name);\n                        has_errors = 1;\n                        continue;\n                }\n\nwould do. See d14e7407b3 (\"needs update\" considered harmful, Sun Jul\n20 00:21:38 2008) for the previous improvement on the subject. The\nproblem with this second patch is that it says \"M\" where \"diff\n--name-status\" would say \"D\" for example, which is a bit strange. If\nthe idea of the patch is accepted, REFRESH_SAY_CHANGED should also be\nrenamed to reflect its new nature, to stg like\nREFRESH_PORCELAIN_OUTPUT.\n\nAny opinion? Am I going in the right direction?\n\n-- \nMatthieu\n"},{"id":"119817","messageId":"7vvdl0kau4.fsf@alter.siamese.dyndns.org","threadId":"20408","inReplyTo":"vpqprb946sk.fsf@bauges.imag.fr","subject":"Re: Message from git reset: confusing?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-06T19:21:07Z","receivedAt":"2009-08-06T19:21:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> This is my biggest issue, indeed. Actually, several things bother me\n> (by decreasing annoyance):\n>\n> 1) It looks like an error message, and user can think git reset\n> failed.\n>\n> 2) It's inconsistant with the usual status display. I'd prefer an\n> output like \"git diff --name-only\" or \"git status\".\n> ...\n> So, let's address 1) and 2) only.\n>\n> Any opinion? Am I going in the right direction?\n\nI think the approach is sane.  I didn't check if the unconditional \n\n\tprintf(\"Unstaged changes after reset:\\n\");\n\nneeds to be protected, or at that point we already know we will talk about\none or more paths (either needs update or needs merge).\n"},{"id":"119926","messageId":"1249676676-5051-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"20408","inReplyTo":"7vvdl0kau4.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] Rename REFRESH_SAY_CHANGED to REFRESH_IN_PORCELAIN.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-07T20:24:35Z","receivedAt":"2009-08-07T20:24:35Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The change in the output is going to become more general than just saying\n\"changed\", so let's make the variable name more general too.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n builtin-add.c   |    2 +-\n builtin-reset.c |    4 ++--\n cache.h         |    2 +-\n read-cache.c    |    2 +-\n 4 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 581a2a1..a325bc9 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -105,7 +105,7 @@ static void refresh(int verbose, const char **pathspec)\n \tfor (specs = 0; pathspec[specs];  specs++)\n \t\t/* nothing */;\n \tseen = xcalloc(specs, 1);\n-\trefresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,\n+\trefresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,\n \t\t      pathspec, seen);\n \tfor (i = 0; i < specs; i++) {\n \t\tif (!seen[i])\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 5fa1789..ddf68d5 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -261,7 +261,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"Cannot do %s reset with paths.\",\n \t\t\t\t\treset_type_names[reset_type]);\n \t\treturn read_from_tree(prefix, argv + i, sha1,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_SAY_CHANGED);\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t}\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n@@ -302,7 +302,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase MIXED: /* Report what has not been updated. */\n \t\tupdate_index_refresh(0, NULL,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_SAY_CHANGED);\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t\tbreak;\n \t}\n \ndiff --git a/cache.h b/cache.h\nindex e6c7f33..a2f2923 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -473,7 +473,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);\n #define REFRESH_QUIET\t\t0x0004\t/* be quiet about it */\n #define REFRESH_IGNORE_MISSING\t0x0008\t/* ignore non-existent */\n #define REFRESH_IGNORE_SUBMODULES\t0x0010\t/* ignore submodules */\n-#define REFRESH_SAY_CHANGED\t0x0020\t/* say \"changed\" not \"needs update\" */\n+#define REFRESH_IN_PORCELAIN\t0x0020\t/* user friendly output, not \"needs update\" */\n extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen);\n \n struct lock_file {\ndiff --git a/read-cache.c b/read-cache.c\nindex 4e3e272..f1aff81 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1077,7 +1077,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n \tconst char *needs_update_message;\n \n-\tneeds_update_message = ((flags & REFRESH_SAY_CHANGED)\n+\tneeds_update_message = ((flags & REFRESH_IN_PORCELAIN)\n \t\t\t\t? \"locally modified\" : \"needs update\");\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tstruct cache_entry *ce, *new;\n-- \n1.6.4.62.g39c83.dirty\n"},{"id":"119925","messageId":"1249676676-5051-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"20408","inReplyTo":"1249676676-5051-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/2 (v2)] reset: make the output more user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-07T20:24:36Z","receivedAt":"2009-08-07T20:24:36Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"git reset without argument displays a summary of the remaining\nunstaged changes. The problem with these is that they look like an\nerror message, and the format is inconsistant with the format used in\nother places like \"git diff --name-status\".\n\nThis patch mimics the output of \"git diff --name-status\", and adds a\nheader to make it clear the output is informative, and not an error.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n\nAs noted by Junio, my previous version did actually display the header\nunconditionnaly. This version displays it as the first change is found.\n\n read-cache.c     |   22 +++++++++++++++++-----\n t/t7102-reset.sh |    3 ++-\n 2 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex f1aff81..4a4f4a5 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1065,6 +1065,15 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \treturn updated;\n }\n \n+static void show_file(const char * fmt, const char * name, int in_porcelain, int * first)\n+{\n+\tif (in_porcelain && *first) {\n+\t\tprintf(\"Unstaged changes after reset:\\n\");\n+\t\t*first=0;\n+\t}\n+\tprintf(fmt, name);\n+}\n+\n int refresh_index(struct index_state *istate, unsigned int flags, const char **pathspec, char *seen)\n {\n \tint i;\n@@ -1074,11 +1083,14 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tint quiet = (flags & REFRESH_QUIET) != 0;\n \tint not_new = (flags & REFRESH_IGNORE_MISSING) != 0;\n \tint ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;\n+\tint first = 1;\n+\tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n-\tconst char *needs_update_message;\n+\tconst char *needs_update_fmt;\n+\tconst char *needs_merge_fmt;\n \n-\tneeds_update_message = ((flags & REFRESH_IN_PORCELAIN)\n-\t\t\t\t? \"locally modified\" : \"needs update\");\n+\tneeds_update_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tneeds_merge_fmt = (in_porcelain ? \"U\\t%s\\n\" : \"%s: needs merge\\n\");\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tstruct cache_entry *ce, *new;\n \t\tint cache_errno = 0;\n@@ -1094,7 +1106,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\ti--;\n \t\t\tif (allow_unmerged)\n \t\t\t\tcontinue;\n-\t\t\tprintf(\"%s: needs merge\\n\", ce->name);\n+\t\t\tshow_file(needs_merge_fmt, ce->name, in_porcelain, &first);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -1117,7 +1129,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\t}\n \t\t\tif (quiet)\n \t\t\t\tcontinue;\n-\t\t\tprintf(\"%s: %s\\n\", ce->name, needs_update_message);\n+\t\t\tshow_file(needs_update_fmt, ce->name, in_porcelain, &first);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex e637c7d..e85ff02 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -419,7 +419,8 @@ test_expect_success 'resetting an unmodified path is a no-op' '\n '\n \n cat > expect << EOF\n-file2: locally modified\n+Unstaged changes after reset:\n+M\tfile2\n EOF\n \n test_expect_success '--mixed refreshes the index' '\n-- \n1.6.4.62.g39c83.dirty\n"},{"id":"119935","messageId":"7viqgztj76.fsf@alter.siamese.dyndns.org","threadId":"20408","inReplyTo":"1249676676-5051-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/2 (v2)] reset: make the output more user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-07T21:20:13Z","receivedAt":"2009-08-07T21:20:13Z","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>  cat > expect << EOF\n> -file2: locally modified\n> +Unstaged changes after reset:\n> +M\tfile2\n\nIt simply feels backwards when plumbing output says something in human\nlanguage (e.g. \"needs update\") while Porcelain output spits out a cryptic\nM or U.  If the goal is human-readability and user-friendliness, shouldn't\nwe rather say:\n\n\tPath with local modifications:\n        \tfile2\n\nor something?\n"},{"id":"119970","messageId":"vpq7hxeu4un.fsf@bauges.imag.fr","threadId":"20408","inReplyTo":"7viqgztj76.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2 (v2)] reset: make the output more user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-08T07:44:48Z","receivedAt":"2009-08-08T07:44:48Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>\n>>  cat > expect << EOF\n>> -file2: locally modified\n>> +Unstaged changes after reset:\n>> +M\tfile2\n>\n> It simply feels backwards when plumbing output says something in human\n> language (e.g. \"needs update\") while Porcelain output spits out a cryptic\n> M or U.  If the goal is human-readability and user-friendliness,\n\nThe goal here is just consistency.\n\nAnd I do consider 'git diff --name-status' as porcelain.\n\n> shouldn't we rather say:\n>\n> \tPath with local modifications:\n>         \tfile2\n>\n> or something?\n\nWhy not, but if we do, we should also remove this \"M\" from other\nplaces. It was already there in one error message given by 'git\nrebase' in a non-clean tree (and you just accepted a patch giving the\nsame output for another one).\n\n-- \nMatthieu\n"},{"id":"120956","messageId":"vpqljlipcs6.fsf@bauges.imag.fr","threadId":"20408","inReplyTo":"vpq7hxeu4un.fsf@bauges.imag.fr","subject":"Re: [PATCH 2/2 (v2)] reset: make the output more user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-17T17:31:53Z","receivedAt":"2009-08-17T17:31:53Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"[ back from holiday ]\n\nAny opinion/update on this? I don't think I got any reply ...\n\nThanks,\n\nMatthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>>\n>>>  cat > expect << EOF\n>>> -file2: locally modified\n>>> +Unstaged changes after reset:\n>>> +M\tfile2\n>>\n>> It simply feels backwards when plumbing output says something in human\n>> language (e.g. \"needs update\") while Porcelain output spits out a cryptic\n>> M or U.  If the goal is human-readability and user-friendliness,\n>\n> The goal here is just consistency.\n>\n> And I do consider 'git diff --name-status' as porcelain.\n>\n>> shouldn't we rather say:\n>>\n>> \tPath with local modifications:\n>>         \tfile2\n>>\n>> or something?\n>\n> Why not, but if we do, we should also remove this \"M\" from other\n> places. It was already there in one error message given by 'git\n> rebase' in a non-clean tree (and you just accepted a patch giving the\n> same output for another one).\n\n-- \nMatthieu\n"},{"id":"120970","messageId":"7vr5vate33.fsf@alter.siamese.dyndns.org","threadId":"20408","inReplyTo":"vpqljlipcs6.fsf@bauges.imag.fr","subject":"Re: [PATCH 2/2 (v2)] reset: make the output more user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-17T19:50:08Z","receivedAt":"2009-08-17T19:50:08Z","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>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>>>\n>>>>  cat > expect << EOF\n>>>> -file2: locally modified\n>>>> +Unstaged changes after reset:\n>>>> +M\tfile2\n>>>\n>>> It simply feels backwards when plumbing output says something in human\n>>> language (e.g. \"needs update\") while Porcelain output spits out a cryptic\n>>> M or U.  If the goal is human-readability and user-friendliness,\n>>\n>> The goal here is just consistency.\n>>\n>> And I do consider 'git diff --name-status' as porcelain.\n\nOk.\n"},{"id":"121403","messageId":"1250845079-30614-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"20408","inReplyTo":"vpqljlipcs6.fsf@bauges.imag.fr","subject":"[PATCH 1/2] Rename REFRESH_SAY_CHANGED to REFRESH_IN_PORCELAIN.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-21T08:57:58Z","receivedAt":"2009-08-21T08:57:58Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The change in the output is going to become more general than just saying\n\"changed\", so let's make the variable name more general too.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n builtin-add.c   |    2 +-\n builtin-reset.c |    4 ++--\n cache.h         |    2 +-\n read-cache.c    |    2 +-\n 4 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 581a2a1..a325bc9 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -105,7 +105,7 @@ static void refresh(int verbose, const char **pathspec)\n \tfor (specs = 0; pathspec[specs];  specs++)\n \t\t/* nothing */;\n \tseen = xcalloc(specs, 1);\n-\trefresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,\n+\trefresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,\n \t\t      pathspec, seen);\n \tfor (i = 0; i < specs; i++) {\n \t\tif (!seen[i])\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 5fa1789..ddf68d5 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -261,7 +261,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"Cannot do %s reset with paths.\",\n \t\t\t\t\treset_type_names[reset_type]);\n \t\treturn read_from_tree(prefix, argv + i, sha1,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_SAY_CHANGED);\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t}\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n@@ -302,7 +302,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase MIXED: /* Report what has not been updated. */\n \t\tupdate_index_refresh(0, NULL,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_SAY_CHANGED);\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t\tbreak;\n \t}\n \ndiff --git a/cache.h b/cache.h\nindex 9222774..ae0e83e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -476,7 +476,7 @@ extern int check_path(const char *path, int len, struct stat *st);\n #define REFRESH_QUIET\t\t0x0004\t/* be quiet about it */\n #define REFRESH_IGNORE_MISSING\t0x0008\t/* ignore non-existent */\n #define REFRESH_IGNORE_SUBMODULES\t0x0010\t/* ignore submodules */\n-#define REFRESH_SAY_CHANGED\t0x0020\t/* say \"changed\" not \"needs update\" */\n+#define REFRESH_IN_PORCELAIN\t0x0020\t/* user friendly output, not \"needs update\" */\n extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen);\n \n struct lock_file {\ndiff --git a/read-cache.c b/read-cache.c\nindex 4e3e272..f1aff81 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1077,7 +1077,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n \tconst char *needs_update_message;\n \n-\tneeds_update_message = ((flags & REFRESH_SAY_CHANGED)\n+\tneeds_update_message = ((flags & REFRESH_IN_PORCELAIN)\n \t\t\t\t? \"locally modified\" : \"needs update\");\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tstruct cache_entry *ce, *new;\n-- \n1.6.4.187.gd399.dirty\n"},{"id":"121402","messageId":"1250845079-30614-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"20408","inReplyTo":"1250845079-30614-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/2 (v3)] reset: make the output more user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-21T08:57:59Z","receivedAt":"2009-08-21T08:57:59Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"git reset without argument displays a summary of the remaining\nunstaged changes. The problem with these is that they look like an\nerror message, and the format is inconsistant with the format used in\nother places like \"git diff --name-status\".\n\nThis patch mimics the output of \"git diff --name-status\", and adds a\nheader to make it clear the output is informative, and not an error.\n\nIt also changes the output of \"git add --refresh --verbose\" in the same\nway.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n\nIn previous version, I hadn't noticed that the same message was\ndisplayed for 'git reset' and for 'git add --refresh --verbose'. This\nnew version takes both cases into account, and avoids saying \"after\nreset\" when the user called \"add\".\n\n builtin-add.c    |    2 +-\n builtin-reset.c  |    3 ++-\n cache.h          |    4 ++--\n read-cache.c     |   26 ++++++++++++++++++++------\n t/t7102-reset.sh |    3 ++-\n 5 files changed, 27 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex a325bc9..006fd08 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -106,7 +106,7 @@ static void refresh(int verbose, const char **pathspec)\n \t\t/* nothing */;\n \tseen = xcalloc(specs, 1);\n \trefresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,\n-\t\t      pathspec, seen);\n+\t\t      pathspec, seen, \"Unstaged changes after refreshing the index:\");\n \tfor (i = 0; i < specs; i++) {\n \t\tif (!seen[i])\n \t\t\tdie(\"pathspec '%s' did not match any files\", pathspec[i]);\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex ddf68d5..0fc0b07 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -108,7 +108,8 @@ static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n \tif (read_cache() < 0)\n \t\treturn error(\"Could not read index\");\n \n-\tresult = refresh_cache(flags) ? 1 : 0;\n+\tresult = refresh_index(&the_index, (flags), NULL, NULL,\n+\t\t\t       \"Unstaged changes after reset:\") ? 1 : 0;\n \tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\tcommit_locked_index(index_lock))\n \t\treturn error (\"Could not refresh index\");\ndiff --git a/cache.h b/cache.h\nindex ae0e83e..fda9816 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -330,7 +330,7 @@ static inline void remove_name_hash(struct cache_entry *ce)\n #define remove_file_from_cache(path) remove_file_from_index(&the_index, (path))\n #define add_to_cache(path, st, flags) add_to_index(&the_index, (path), (st), (flags))\n #define add_file_to_cache(path, flags) add_file_to_index(&the_index, (path), (flags))\n-#define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL)\n+#define refresh_cache(flags) refresh_index(&the_index, (flags), NULL, NULL, NULL)\n #define ce_match_stat(ce, st, options) ie_match_stat(&the_index, (ce), (st), (options))\n #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))\n #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))\n@@ -477,7 +477,7 @@ extern int check_path(const char *path, int len, struct stat *st);\n #define REFRESH_IGNORE_MISSING\t0x0008\t/* ignore non-existent */\n #define REFRESH_IGNORE_SUBMODULES\t0x0010\t/* ignore submodules */\n #define REFRESH_IN_PORCELAIN\t0x0020\t/* user friendly output, not \"needs update\" */\n-extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen);\n+extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen, char *header_msg);\n \n struct lock_file {\n \tstruct lock_file *next;\ndiff --git a/read-cache.c b/read-cache.c\nindex f1aff81..1bbaf1c 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1065,7 +1065,18 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \treturn updated;\n }\n \n-int refresh_index(struct index_state *istate, unsigned int flags, const char **pathspec, char *seen)\n+static void show_file(const char * fmt, const char * name, int in_porcelain,\n+\t\t      int * first, char *header_msg)\n+{\n+\tif (in_porcelain && *first && header_msg) {\n+\t\tprintf(\"%s\\n\", header_msg);\n+\t\t*first=0;\n+\t}\n+\tprintf(fmt, name);\n+}\n+\n+int refresh_index(struct index_state *istate, unsigned int flags, const char **pathspec,\n+\t\t  char *seen, char *header_msg)\n {\n \tint i;\n \tint has_errors = 0;\n@@ -1074,11 +1085,14 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tint quiet = (flags & REFRESH_QUIET) != 0;\n \tint not_new = (flags & REFRESH_IGNORE_MISSING) != 0;\n \tint ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;\n+\tint first = 1;\n+\tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n-\tconst char *needs_update_message;\n+\tconst char *needs_update_fmt;\n+\tconst char *needs_merge_fmt;\n \n-\tneeds_update_message = ((flags & REFRESH_IN_PORCELAIN)\n-\t\t\t\t? \"locally modified\" : \"needs update\");\n+\tneeds_update_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tneeds_merge_fmt = (in_porcelain ? \"U\\t%s\\n\" : \"%s: needs merge\\n\");\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tstruct cache_entry *ce, *new;\n \t\tint cache_errno = 0;\n@@ -1094,7 +1108,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\ti--;\n \t\t\tif (allow_unmerged)\n \t\t\t\tcontinue;\n-\t\t\tprintf(\"%s: needs merge\\n\", ce->name);\n+\t\t\tshow_file(needs_merge_fmt, ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -1117,7 +1131,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\t}\n \t\t\tif (quiet)\n \t\t\t\tcontinue;\n-\t\t\tprintf(\"%s: %s\\n\", ce->name, needs_update_message);\n+\t\t\tshow_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex e637c7d..e85ff02 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -419,7 +419,8 @@ test_expect_success 'resetting an unmodified path is a no-op' '\n '\n \n cat > expect << EOF\n-file2: locally modified\n+Unstaged changes after reset:\n+M\tfile2\n EOF\n \n test_expect_success '--mixed refreshes the index' '\n-- \n1.6.4.187.gd399.dirty\n"},{"id":"121501","messageId":"7v3a7k767j.fsf@alter.siamese.dyndns.org","threadId":"20408","inReplyTo":"1250845079-30614-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/2 (v3)] reset: make the output more user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-22T05:44:48Z","receivedAt":"2009-08-22T05:44:48Z","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> git reset without argument displays a summary of the remaining\n> unstaged changes. The problem with these is that they look like an\n> error message, and the format is inconsistant with the format used in\n> other places like \"git diff --name-status\".\n>\n> This patch mimics the output of \"git diff --name-status\", and adds a\n> header to make it clear the output is informative, and not an error.\n>\n> It also changes the output of \"git add --refresh --verbose\" in the same\n> way.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nThanks.  Will queue.\n\nHowever, I'd change the justification.\n\n    git reset without argument displays a summary of the local modification,\n    like this:\n    \n        $ git reset\n        Makefile: locally modified\n    \n    Some people have problems with this; they look like an error message.\n    \n    This patch makes its output mimic how \"git checkout $another_branch\"\n    reports the paths with local modifications.  \"git add --refresh --verbose\"\n    is changed in the same way.\n    \n    It also adds a header to make it clear that the output is informative,\n    and not an error.\n    \n    Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nThe output from reset in question is merely an informative side effect, as\nopposed to what you actively ask \"git diff\" to give as its primary output.\nAs such, your \"consistency\" argument is pretty weak.  There is no reason\nto expect that the informative message to resemble one particular format\n(namely, --name-status) and not another (e.g. --stat or --name-only), as\nyou are not explicitly specifying what format to use; nor we would want to\nmake it customizable--after all it is just a friendly reminder.\n\nInformative output from \"git checkout $branch\" when there are local\nchanges is a much better precedent to refer to.\n\nAfter applying your patch and having compared these two sets of output:\n\n        (1) without changes\n\n        $ git reset --hard\n        $ git checkout mm/reset-report\n        Already on 'mm/reset-report'\n        $ git reset\n        $ git add --refresh -v Makefile\n\n        (2) with changes\n\n        $ echo >>Makefile\n        $ git add Makefile\n        $ git checkout mm/reset-report\n        M       Makefile\n        Already on 'mm/reset-report'\n        $ git reset\n        Unstaged changes after reset:\n        M       Makefile\n        $ git add --refresh -v Makefile\n        Unstaged changes after refreshing the index:\n        M       Makefile\n\nI am somewhat inclined to suggest that we should drop the new \"Unstaged\nchanges after ...\" message, though.\n\nBy the way, \"Already on .../Switched to ...\" noise from \"git checkout\" is\nalso very annoying.  It is useless to report that the command did exactly\nwhat the user told it to do.  Even more annoyingly, \"git checkout -q\" to\nsquelch this useless noise also squelches the \"here are the paths you have\nlocal changes\" reminder, which is much more useful.\n\nBut that is a separate topic.\n"},{"id":"121507","messageId":"vpq8whc8euu.fsf@bauges.imag.fr","threadId":"20408","inReplyTo":"7v3a7k767j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2 (v3)] reset: make the output more user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-22T07:52:41Z","receivedAt":"2009-08-22T07:52:41Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Thanks.  Will queue.\n\nThanks,\n\n> However, I'd change the justification.\n\nFine with me.\n\n> The output from reset in question is merely an informative side effect, as\n> opposed to what you actively ask \"git diff\" to give as its primary output.\n> As such, your \"consistency\" argument is pretty weak.  There is no reason\n> to expect that the informative message to resemble one particular format\n> (namely, --name-status) and not another (e.g. --stat or --name-only),\n\nI agree that chosing --name-status over, like, --stat is rather\narbitrary. But Git has IMHO far too many languages for talking about\nchanges (--stat, --name-status, 'git status' itself, the 'git ls-files\n-t' that I just discovered, and this 'bla: locally modified').\nReducing the number of formats by one is a good thing to me.\n\n> Informative output from \"git checkout $branch\" when there are local\n> changes is a much better precedent to refer to.\n\nYes.\n\n> I am somewhat inclined to suggest that we should drop the new \"Unstaged\n> changes after ...\" message, though.\n\nI've thought about this too. The new format already looks much less\nlike an error message, which was really the problem I was solving. But\none advantage of the message contains two relevant informations:\n\"unstaged\" and \"after\".\n\nIntuitively, I would have thought that \"git reset\" was reporting what\nit was doing, as it was doing it. So to me (before experimenting a bit\nmore and looking at the source code),\n\nM\tfoo.txt\nM\tbar.txt\n\nwould mean \"I've just reseted foo.txt and bar.txt, which were locally\nmodified\", while actually \"git reset\" can very well show this message\nafter reseting only foo.txt, just informing the user that bar.txt is\nalso modified. So, at least to me, the semantics was very unclear, and\nwhile I would have understood immediately with the one-liner message.\n\nIn short: no strong objection to remove this message, but to me it is\nusefull.\n\n-- \nMatthieu\n"},{"id":"121547","messageId":"7vmy5r1cpo.fsf@alter.siamese.dyndns.org","threadId":"20408","inReplyTo":"vpq8whc8euu.fsf@bauges.imag.fr","subject":"Re: [PATCH 2/2 (v3)] reset: make the output more user-friendly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T02:33:07Z","receivedAt":"2009-08-23T02:33: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> Intuitively, I would have thought that \"git reset\" was reporting what\n> it was doing, as it was doing it. So to me (before experimenting a bit\n> more and looking at the source code),\n>\n> M\tfoo.txt\n> M\tbar.txt\n>\n> would mean \"I've just reseted foo.txt and bar.txt, which were locally\n> modified\", while actually \"git reset\" can very well show this message\n> after reseting only foo.txt, just informing the user that bar.txt is\n> also modified. So, at least to me, the semantics was very unclear, and\n> while I would have understood immediately with the one-liner message.\n>\n> In short: no strong objection to remove this message, but to me it is\n> usefull.\n\nThanks for sharing the reasoning.\n\nTwo conflicting/competing thoughts come to mind:\n\n 1. Perhaps we should add a similar \"explanation\" for the list of paths\n    with changes upon switching branches with \"git checkout\" for\n    consistency.\n\n 2. Such an \"explanation\" of what the output means would help the first\n    time people, but would everybody stay \"first time\" forever?  Would the\n    explanation become just another wasted line in valuable screen real\n    estate after people gain experience?\n\nI am leaning towards #1 right now.\n"},{"id":"121561","messageId":"vpqiqgevmju.fsf@bauges.imag.fr","threadId":"20408","inReplyTo":"7vmy5r1cpo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2 (v3)] reset: make the output more user-friendly.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-23T10:42:29Z","receivedAt":"2009-08-23T10:42:29Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Two conflicting/competing thoughts come to mind:\n>\n>  1. Perhaps we should add a similar \"explanation\" for the list of paths\n>     with changes upon switching branches with \"git checkout\" for\n>     consistency.\n\nActually, I had never paid much attention to this message for\ncheckout. Just checked, and I got it wrong too ;-). I thought checkout\nwas showing me the files it was modifying, that wasn't it.\n\nThat said, I'm not a heavy user of local branches, so I'm a bad judge\non what should be the behavior.\n\n>  2. Such an \"explanation\" of what the output means would help the first\n>     time people, but would everybody stay \"first time\" forever?  Would the\n>     explanation become just another wasted line in valuable screen real\n>     estate after people gain experience?\n\nYes, and this is a much more general issue than just checkout/reset.\nFor example, the output of 'git status' is very nice to newbies:\n\n  # On branch master\n  # Changed but not updated:\n  #   (use \"git add <file>...\" to update what will be committed)\n  #   (use \"git checkout -- <file>...\" to discard changes in working directory)\n  #\n  #       modified:   git.c\n  #\n  no changes added to commit (use \"git add\" and/or \"git commit -a\")\n\nBut out of these 8 lines, only two contain real informations, and the\n(use \"git bla\") are just noise to expert users.\n\nI've been thinking of a configuration option, like \"core.expertuser\"\nor \"ui.expertuser\" that would let users disable these informative\nmessages on demand. I'm not sure how good the idea is.\n\n--\nMatthieu\n"},{"id":"121563","messageId":"3f4fd2640908230445m427175f6v209d695e7e31b303@mail.gmail.com","threadId":"20408","inReplyTo":"vpqiqgevmju.fsf@bauges.imag.fr","subject":"Re: [PATCH 2/2 (v3)] reset: make the output more user-friendly.","fromName":"Reece Dunn","fromEmail":"msclrhd@googlemail.com","sentAt":"2009-08-23T11:45:18Z","receivedAt":"2009-08-23T11:45:18Z","isPatch":true,"sender":{"key":"msclrhd@googlemail.com","avatar":null},"body":"2009/8/23 Matthieu Moy <Matthieu.Moy@imag.fr>:\n> For example, the output of 'git status' is very nice to newbies:\n>\n>  # On branch master\n>  # Changed but not updated:\n>  #   (use \"git add <file>...\" to update what will be committed)\n\nShouldn't this be something like: (use \"git add <file>...\" to add new\nand modified files to be committed) -- I am saying this as \"update\"\ncan also refer to removing files, or discarding changes.\n\n>  #   (use \"git checkout -- <file>...\" to discard changes in working directory)\n>  #\n>  #       modified:   git.c\n>  #\n>  no changes added to commit (use \"git add\" and/or \"git commit -a\")\n>\n> But out of these 8 lines, only two contain real informations, and the\n> (use \"git bla\") are just noise to expert users.\n\nYes and no. Using git for quite a while now, the day-to-day operations\nare second nature, but other slightly obscure commands (how exactly do\nI remove a staged file?) are useful to have.\n\n> I've been thinking of a configuration option, like \"core.expertuser\"\n> or \"ui.expertuser\" that would let users disable these informative\n> messages on demand. I'm not sure how good the idea is.\n\nThe \"core.expertuser\" option does not really say what this is doing\n(should the expertuser option list the sha1's for the commits, trees\nand objects it is adding?).\n\nI would call it something like \"core.interactive-help\",\n\"core.inplace-help\" or \"core.inline-help\", as that is what the (use\n...) lines are.\n\n- Reece\n"}]}