{"thread":{"id":"28869","subject":"reset reports file as modified when it's in fact deleted","startedAt":"2011-11-07T09:43:30Z","lastAt":"2011-11-18T20:40:29Z","messageCount":18,"participants":["Carlos Martín Nieto","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"179016","messageId":"20111107094330.GB10936@beez.lab.cmartin.tk","threadId":"28869","inReplyTo":null,"subject":"reset reports file as modified when it's in fact deleted","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-11-07T09:43:30Z","receivedAt":"2011-11-07T09:43:30Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Hello,\n\nWhen I delete a file (git rm) and then reset so it exists in the index\nagain, the message tells me 'M file.txt' even though the file doesn't\nexist in the worktree anymore. Running git status afterwards does give\nthe correct output. So, here's the minimal steps to reproduce:\n\n    $ git init\n    Initialized empty Git repository in /home/carlos/test/reset-err/.git/\n    $ touch file.txt\n    $ git add file.txt\n    $ git ci file.txt -m initial\n    [master (root-commit) a536393] initial\n     0 files changed, 0 insertions(+), 0 deletions(-)\n     create mode 100644 file.txt\n    $ git rm file.txt\n    rm 'file.txt'\n    $ git reset -- file.txt\n    Unstaged changes after reset:\n    M\t\t file.txt\n    $ git status -b -s\n    ## master\n     D file.txt\n\nI'd expect the output after \"Unstaged changes after reset\" to tell me\nfile.txt has been deleted instead of modified. This happens with\n1.7.8-rc0, 1.7.7 and 1.7.4.1 and I expect with many more that I don't\nhave here.\n\nI thought the index diff code might have been checking the index at the\nwrong time, but I can run 'git reset HEAD -- file.txt' as many times\nas I want, and it will still say 'M', so now I'm not sure.\n\n   cmn\n"},{"id":"179043","messageId":"20111107162642.GA27055@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111107094330.GB10936@beez.lab.cmartin.tk","subject":"Re: reset reports file as modified when it's in fact deleted","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-07T16:26:42Z","receivedAt":"2011-11-07T16:26:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 07, 2011 at 10:43:30AM +0100, Carlos Martín Nieto wrote:\n\n> When I delete a file (git rm) and then reset so it exists in the index\n> again, the message tells me 'M file.txt' even though the file doesn't\n> exist in the worktree anymore. Running git status afterwards does give\n> the correct output. So, here's the minimal steps to reproduce:\n> \n>     $ git init\n>     Initialized empty Git repository in /home/carlos/test/reset-err/.git/\n>     $ touch file.txt\n>     $ git add file.txt\n>     $ git ci file.txt -m initial\n>     [master (root-commit) a536393] initial\n>      0 files changed, 0 insertions(+), 0 deletions(-)\n>      create mode 100644 file.txt\n>     $ git rm file.txt\n>     rm 'file.txt'\n>     $ git reset -- file.txt\n>     Unstaged changes after reset:\n>     M\t\t file.txt\n>     $ git status -b -s\n>     ## master\n>      D file.txt\n\nYou can simplify this even further by not touching the index at all:\n\n  git init -q &&\n  >file && git add file && git commit -q -m initial &&\n  rm file &&\n  git reset\n\nproduces:\n\n  Unstaged changes after reset:\n  M       file\n\n> I'd expect the output after \"Unstaged changes after reset\" to tell me\n> file.txt has been deleted instead of modified. This happens with\n> 1.7.8-rc0, 1.7.7 and 1.7.4.1 and I expect with many more that I don't\n> have here.\n> \n> I thought the index diff code might have been checking the index at the\n> wrong time, but I can run 'git reset HEAD -- file.txt' as many times\n> as I want, and it will still say 'M', so now I'm not sure.\n\nThe index diff code isn't running at all. Those messages are produced by\nrefresh_index, which outputs only two flags: modified or unmerged. I\nthink the reason for this is somewhat historical. You would run\n\"update-index --refresh\", and it would helpfully say \"by the way, when\nrefreshing this entry, I noticed that it is in need of being updated in\nthe index\". The text was \"file.txt: needs update\".\n\nLater, many porcelain commands started to refresh the index themselves,\nand the \"needs update\" message was very confusing. So it was switched to\nthe more familiar \"M file.txt\" (though you can still see the original\nplumbing message if you run update-index yourself).\n\nI think it is a little more friendly to distinguish deletion from just\nmodification. And there's shouldn't be any compatibility questions, as\nthese are explicitly porcelain output (plumbing will still say \"needs\nupdate\").\n\nThere are a few other cases users might expect to see. We'll never show\ncopies or renames, of course, because we aren't actually doing a diff\nwith copy detection. We won't see an \"A\"dded file, because such a file\nwould be in the working tree but not the index, meaning it is not\ntracked.\n\nWe could see a \"T\"ypechange, but the distinction between that and a\nmodified file is lost by the time we get to refresh_index. We could pass\nit up, but I wonder if it's really worth it.\n\nThe patch to do \"D\"eleted is pretty simple:\n\ndiff --git a/read-cache.c b/read-cache.c\nindex dea7cd8..cc1ebdf 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n \tconst char *needs_update_fmt;\n+\tconst char *needs_rm_fmt;\n \tconst char *needs_merge_fmt;\n \n \tneeds_update_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tneeds_rm_fmt = (in_porcelain ? \"D\\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@@ -1145,7 +1147,10 @@ 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\tshow_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\tif (cache_errno == ENOENT)\n+\t\t\t\tshow_file(needs_rm_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\telse\n+\t\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}\n"},{"id":"179316","messageId":"20111111140834.GA10025@beez.lab.cmartin.tk","threadId":"28869","inReplyTo":"20111107162642.GA27055@sigill.intra.peff.net","subject":"Re: reset reports file as modified when it's in fact deleted","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-11-11T14:08:34Z","receivedAt":"2011-11-11T14:08:34Z","isPatch":false,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:\n> On Mon, Nov 07, 2011 at 10:43:30AM +0100, Carlos Martín Nieto wrote:\n> \n> > When I delete a file (git rm) and then reset so it exists in the index\n> > again, the message tells me 'M file.txt' even though the file doesn't\n> > exist in the worktree anymore. Running git status afterwards does give\n> > the correct output. So, here's the minimal steps to reproduce:\n> > \n> >     $ git init\n> >     Initialized empty Git repository in /home/carlos/test/reset-err/.git/\n> >     $ touch file.txt\n> >     $ git add file.txt\n> >     $ git ci file.txt -m initial\n> >     [master (root-commit) a536393] initial\n> >      0 files changed, 0 insertions(+), 0 deletions(-)\n> >      create mode 100644 file.txt\n> >     $ git rm file.txt\n> >     rm 'file.txt'\n> >     $ git reset -- file.txt\n> >     Unstaged changes after reset:\n> >     M\t\t file.txt\n> >     $ git status -b -s\n> >     ## master\n> >      D file.txt\n> \n> You can simplify this even further by not touching the index at all:\n> \n>   git init -q &&\n>   >file && git add file && git commit -q -m initial &&\n>   rm file &&\n>   git reset\n> \n> produces:\n> \n>   Unstaged changes after reset:\n>   M       file\n\nAh, I see. I got the previous sequence because that's what we have in\nan instruction manual and where we saw it.\n\n> \n> > I'd expect the output after \"Unstaged changes after reset\" to tell me\n> > file.txt has been deleted instead of modified. This happens with\n> > 1.7.8-rc0, 1.7.7 and 1.7.4.1 and I expect with many more that I don't\n> > have here.\n> > \n> > I thought the index diff code might have been checking the index at the\n> > wrong time, but I can run 'git reset HEAD -- file.txt' as many times\n> > as I want, and it will still say 'M', so now I'm not sure.\n> \n> The index diff code isn't running at all. Those messages are produced by\n> refresh_index, which outputs only two flags: modified or unmerged. I\n> think the reason for this is somewhat historical. You would run\n> \"update-index --refresh\", and it would helpfully say \"by the way, when\n> refreshing this entry, I noticed that it is in need of being updated in\n> the index\". The text was \"file.txt: needs update\".\n> \n> Later, many porcelain commands started to refresh the index themselves,\n> and the \"needs update\" message was very confusing. So it was switched to\n> the more familiar \"M file.txt\" (though you can still see the original\n> plumbing message if you run update-index yourself).\n> \n> I think it is a little more friendly to distinguish deletion from just\n> modification. And there's shouldn't be any compatibility questions, as\n> these are explicitly porcelain output (plumbing will still say \"needs\n> update\").\n> \n> There are a few other cases users might expect to see. We'll never show\n> copies or renames, of course, because we aren't actually doing a diff\n> with copy detection. We won't see an \"A\"dded file, because such a file\n> would be in the working tree but not the index, meaning it is not\n> tracked.\n> \n> We could see a \"T\"ypechange, but the distinction between that and a\n> modified file is lost by the time we get to refresh_index. We could pass\n> it up, but I wonder if it's really worth it.\n\nThat's probably overkill. The issue with reporting 'M' for a deleted\nfile is that it conflicts with what the status output would say, even\nthough it's in the same format.\n\n> \n> The patch to do \"D\"eleted is pretty simple:\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index dea7cd8..cc1ebdf 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n>  \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n>  \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n>  \tconst char *needs_update_fmt;\n> +\tconst char *needs_rm_fmt;\n>  \tconst char *needs_merge_fmt;\n>  \n>  \tneeds_update_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n> +\tneeds_rm_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n>  \tneeds_merge_fmt = (in_porcelain ? \"U\\t%s\\n\" : \"%s: needs merge\\n\");\n\nWhile the name fits in with the rest of the variables, it's kind of\nthe wrong way around, isn't it? It doesn't need an 'rm', it /was/\nrm'd. Other than that stupid thing, the patch works great, thanks. Can\nwe get it into git?\n\n>  \tfor (i = 0; i < istate->cache_nr; i++) {\n>  \t\tstruct cache_entry *ce, *new;\n> @@ -1145,7 +1147,10 @@ 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\tshow_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);\n> +\t\t\tif (cache_errno == ENOENT)\n> +\t\t\t\tshow_file(needs_rm_fmt, ce->name, in_porcelain, &first, header_msg);\n> +\t\t\telse\n> +\t\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}\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"179321","messageId":"7vr51er4nd.fsf@alter.siamese.dyndns.org","threadId":"28869","inReplyTo":"20111111140834.GA10025@beez.lab.cmartin.tk","subject":"Re: reset reports file as modified when it's in fact deleted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-11T16:43:34Z","receivedAt":"2011-11-11T16:43:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto <cmn@elego.de> writes:\n\n> On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:\n> ...\n>> The patch to do \"D\"eleted is pretty simple:\n>> \n>> diff --git a/read-cache.c b/read-cache.c\n>> index dea7cd8..cc1ebdf 100644\n>> --- a/read-cache.c\n>> +++ b/read-cache.c\n>> @@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n>>  \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n>>  \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n>>  \tconst char *needs_update_fmt;\n>> +\tconst char *needs_rm_fmt;\n>>  \tconst char *needs_merge_fmt;\n>>  \n>>  \tneeds_update_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n>> +\tneeds_rm_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n>>  \tneeds_merge_fmt = (in_porcelain ? \"U\\t%s\\n\" : \"%s: needs merge\\n\");\n>\n> While the name fits in with the rest of the variables, it's kind of\n> the wrong way around, isn't it? It doesn't need an 'rm', it /was/\n> rm'd.\n\nThe variable names were chosen to mean \"In a situation where the plumbing\ntraditionally would have said X, use this format to describe it\". This is\nthe first topic to separate a single situation (from the plumbing's point\nof view) into two and say different things at Porcelain, and the variable\nnaming no longer works.\n\nAn obvious solution would be to rename all of them to be based on \"what\nhappened to the path\". E.g. \"modified_fmt\" would be set to either \"M\" or\n\"needs update\", and \"removed_fmt\" would be set to either \"D\" or \"needs\nupdate\", etc.\n"},{"id":"179323","messageId":"20111111182109.GB16055@sigill.intra.peff.net","threadId":"28869","inReplyTo":"7vr51er4nd.fsf@alter.siamese.dyndns.org","subject":"Re: reset reports file as modified when it's in fact deleted","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-11T18:21:09Z","receivedAt":"2011-11-11T18:21:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 11, 2011 at 08:43:34AM -0800, Junio C Hamano wrote:\n\n> Carlos Martín Nieto <cmn@elego.de> writes:\n> \n> > On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:\n> > ...\n> >> The patch to do \"D\"eleted is pretty simple:\n> >> \n> >> diff --git a/read-cache.c b/read-cache.c\n> >> index dea7cd8..cc1ebdf 100644\n> >> --- a/read-cache.c\n> >> +++ b/read-cache.c\n> >> @@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n> >>  \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n> >>  \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n> >>  \tconst char *needs_update_fmt;\n> >> +\tconst char *needs_rm_fmt;\n> >>  \tconst char *needs_merge_fmt;\n> >>  \n> >>  \tneeds_update_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n> >> +\tneeds_rm_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n> >>  \tneeds_merge_fmt = (in_porcelain ? \"U\\t%s\\n\" : \"%s: needs merge\\n\");\n> >\n> > While the name fits in with the rest of the variables, it's kind of\n> > the wrong way around, isn't it? It doesn't need an 'rm', it /was/\n> > rm'd.\n> \n> The variable names were chosen to mean \"In a situation where the plumbing\n> traditionally would have said X, use this format to describe it\". This is\n> the first topic to separate a single situation (from the plumbing's point\n> of view) into two and say different things at Porcelain, and the variable\n> naming no longer works.\n\nI agree the naming is awkward (but then, I think the \"needs update\"\nmessage is awkward ;) ). But my interpretation was: if you want the\nindex to be in sync with the working tree, you must do this. Hence:\n\n  $EDITOR file             ;# triggers needs_update\n  git update-index file    ;# and do update in index\n  rm file                  ;# triggers needs rm\n  git rm --cached file     ;# and do rm in index\n\nSo that was my attempt to follow the same scheme, and I think it does\nwork. But I agree it's awkward, and since we're not changing the\nplumbing message (nor do I think we should; most users won't see it, and\nwe would only be breaking scripts that do), it's not a big deal just to\nchange the variable names.\n\n> An obvious solution would be to rename all of them to be based on \"what\n> happened to the path\". E.g. \"modified_fmt\" would be set to either \"M\" or\n> \"needs update\", and \"removed_fmt\" would be set to either \"D\" or \"needs\n> update\", etc.\n\nI'm happy to make that change. What do you think of the feature overall?\nAnd should typechanges also be handled here (it's no extra work for git\nto detect them; we just have to pass the TYPE_CHANGED flag back up the\nstack)?\n\n-Peff\n"},{"id":"179340","messageId":"7vhb2aqjwu.fsf@alter.siamese.dyndns.org","threadId":"28869","inReplyTo":"20111111182109.GB16055@sigill.intra.peff.net","subject":"Re: reset reports file as modified when it's in fact deleted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-12T00:11:29Z","receivedAt":"2011-11-12T00:11:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Nov 11, 2011 at 08:43:34AM -0800, Junio C Hamano wrote:\n>\n> I agree the naming is awkward (but then, I think the \"needs update\"\n> message is awkward ;) ). But my interpretation was: if you want the\n> index to be in sync with the working tree, you must do this. Hence:\n>\n>   $EDITOR file             ;# triggers needs_update\n>   git update-index file    ;# and do update in index\n>   rm file                  ;# triggers needs rm\n>   git rm --cached file     ;# and do rm in index\n\nThe word \"update\" at the plumbing level simply means \"update the path in\nthe index\". At the Porcelain level you have to use \"git add\" or \"git rm\"\ndepending on different kind of \"needs update\", which may be argued an\ninconsistency, but at the plumbing level everything is done with the same\nupdate-index command, and \"needs update\" is the word that matches your\ninterpretation.\n\nI however think the topic was about updating the Porclain-ish output that\nis similar to \"status -s\" output. And at that level, paths marked with M/T\nand D are dealt differently by end users (\"add\" vs \"rm\").\n\nAnd the ? : expressions the patch is touching is about mapping the state\nof the path to your \"you must do this\" interpretation. What is being\nmapped is the state, so it would be more natural to have the variable\n\"modified_fmt\" hold the result of the mapping \"you must do...\", instead of\ncalling the variable in terms of \"you must do this if you were working at\nthe plumbing level\", which is where the original needs_update_fmt naming\ncomes from.\n\n>> An obvious solution would be to rename all of them to be based on \"what\n>> happened to the path\". E.g. \"modified_fmt\" would be set to either \"M\" or\n>> \"needs update\", and \"removed_fmt\" would be set to either \"D\" or \"needs\n>> update\", etc.\n>\n> I'm happy to make that change. What do you think of the feature overall?\n\nThe \"feature\" is more or less \"Meh\" to me; I do not mind differentiating M\nand D there because the necessary information is already there in the\ncodepath, but if we were to really do that, then the variables must be\nrenamed. We shouldn't name them after \"you must do this at the plumbing\nlevel\" like we have done so far, and instead use \"the path is in this\nstate\". This is even more so if we were to further split a single \"state\"\nthat plumbing layer considers the same and gives the same \"needs X\" to\ndifferent states that Porcelains would give different messages in the\nfuture.\n\n> And should typechanges also be handled here (it's no extra work for git\n> to detect them; we just have to pass the TYPE_CHANGED flag back up the\n> stack)?\n\nAs long as \"pass ... back up the stack\" does not result in too much\nugliness in the code, I am OK with it.\n"},{"id":"179454","messageId":"20111114225056.GA27370@sigill.intra.peff.net","threadId":"28869","inReplyTo":"7vhb2aqjwu.fsf@alter.siamese.dyndns.org","subject":"Re: reset reports file as modified when it's in fact deleted","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-14T22:50:56Z","receivedAt":"2011-11-14T22:50:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 11, 2011 at 04:11:29PM -0800, Junio C Hamano wrote:\n\n> > I'm happy to make that change. What do you think of the feature overall?\n> \n> The \"feature\" is more or less \"Meh\" to me; I do not mind differentiating M\n> and D there because the necessary information is already there in the\n> codepath, but if we were to really do that, then the variables must be\n> renamed. We shouldn't name them after \"you must do this at the plumbing\n> level\" like we have done so far, and instead use \"the path is in this\n> state\". This is even more so if we were to further split a single \"state\"\n> that plumbing layer considers the same and gives the same \"needs X\" to\n> different states that Porcelains would give different messages in the\n> future.\n\nI admit I don't care all that much either, but it's easy to do, and I\nthink it is less surprising to users. Patches are below.\n\n> > And should typechanges also be handled here (it's no extra work for git\n> > to detect them; we just have to pass the TYPE_CHANGED flag back up the\n> > stack)?\n> \n> As long as \"pass ... back up the stack\" does not result in too much\n> ugliness in the code, I am OK with it.\n\nIt ended up pretty simple, but I split it out from the deletion case so\nyou can see it more clearly (and can drop the latter half of the series\nif we don't want it).\n\n  [1/4]: refresh_index: rename format variables\n  [2/4]: refresh_index: mark deletions in porcelain output\n  [3/4]: read-cache: let refresh_cache_ent pass up changed flags\n  [4/4]: refresh_index: notice typechanges in output\n\n(If I were sure we wanted the typechange output, I think a more\nreasonable ordering would be 1, then 3, then squash 2 and 4 into a\nsingle patch. But see my comment in 4/4).\n\n-Peff\n"},{"id":"179455","messageId":"20111114225153.GA3993@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111114225056.GA27370@sigill.intra.peff.net","subject":"[PATCH 1/4] refresh_index: rename format variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-14T22:51:53Z","receivedAt":"2011-11-14T22:51:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When refreshing the index, for modified (or unmerged) files\nwe will print \"needs update\" (or \"needs merge\") for plumbing,\nor a \"diff --name-status\"-ish line for porcelain.\n\nThe variables holding which type of message to show are\nnamed after the plumbing messages. However, as we begin to\ndifferentiate more cases at the porcelain level (with the\nplumbing message stayin the same), that naming scheme will\nbecome awkward.\n\nInstead, name the variables after which case we found\n(modified or unmerged), not what we will output.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n read-cache.c |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 5790a91..eb3aae3 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1102,11 +1102,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\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_fmt;\n-\tconst char *needs_merge_fmt;\n+\tconst char *modified_fmt;\n+\tconst char *unmerged_fmt;\n \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+\tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tunmerged_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@@ -1122,7 +1122,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\tshow_file(needs_merge_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\tshow_file(unmerged_fmt, ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -1145,7 +1145,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\tshow_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\tshow_file(modified_fmt, ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179456","messageId":"20111114225208.GB3993@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111114225056.GA27370@sigill.intra.peff.net","subject":"[PATCH 2/4] refresh_index: mark deletions in porcelain output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-14T22:52:08Z","receivedAt":"2011-11-14T22:52:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If you have a deleted file and a porcelain refreshes the\ncache, we print:\n\n  Unstaged changes after reset:\n  M\tfile\n\nThis is technically correct, in that the file is modified,\nbut it's friendlier to the user if we further differentiate\nthe case of a deleted file (especially because this output\nlooks a lot like \"diff --name-status\", which would also make\nthe distinction).\n\nThe plumbing version stays as \"needs update\" for historical\ncompatibility.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n read-cache.c |    6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex eb3aae3..046cf7e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n \tconst char *modified_fmt;\n+\tconst char *deleted_fmt;\n \tconst char *unmerged_fmt;\n \n \tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tdeleted_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n \tunmerged_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@@ -1145,7 +1147,9 @@ 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\tshow_file(modified_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\tshow_file((cache_errno == ENOENT ? deleted_fmt :\n+\t\t\t\t   modified_fmt),\n+\t\t\t\t  ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179457","messageId":"20111114225245.GC3993@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111114225056.GA27370@sigill.intra.peff.net","subject":"[PATCH 3/4] read-cache: let refresh_cache_ent pass up changed flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-14T22:52:45Z","receivedAt":"2011-11-14T22:52:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This will enable refresh_cache to differentiate more cases\nof modification (such as typechange) when telling the user\nwhat isn't fresh.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThere's only a single layer of call-stack to modify, and there's only\none other caller, so it turned out to be a pretty minor change.\n\n read-cache.c |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 046cf7e..83fb19c 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1001,7 +1001,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n  */\n static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \t\t\t\t\t     struct cache_entry *ce,\n-\t\t\t\t\t     unsigned int options, int *err)\n+\t\t\t\t\t     unsigned int options, int *err,\n+\t\t\t\t\t     int *changed_ret)\n {\n \tstruct stat st;\n \tstruct cache_entry *updated;\n@@ -1033,6 +1034,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n \t}\n \n \tchanged = ie_match_stat(istate, ce, &st, options);\n+\tif (changed_ret)\n+\t\t*changed_ret = changed;\n \tif (!changed) {\n \t\t/*\n \t\t * The path is unchanged.  If we were told to ignore\n@@ -1132,7 +1135,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\tif (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n \t\t\tcontinue;\n \n-\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno);\n+\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);\n \t\tif (new == ce)\n \t\t\tcontinue;\n \t\tif (!new) {\n@@ -1161,7 +1164,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)\n {\n-\treturn refresh_cache_ent(&the_index, ce, really, NULL);\n+\treturn refresh_cache_ent(&the_index, ce, really, NULL, NULL);\n }\n \n static int verify_hdr(struct cache_header *hdr, unsigned long size)\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179458","messageId":"20111114225651.GD3993@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111114225056.GA27370@sigill.intra.peff.net","subject":"[PATCH 4/4] refresh_index: notice typechanges in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-14T22:56:51Z","receivedAt":"2011-11-14T22:56:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If a file changes type and a porcelain updates the index, we\nwill print \"M file\". Instead, let's be more specific and\nprint \"T file\", which matches actual diff and status output.\n\nThe plumbing version remains \"needs update\" for historical\ncompatibility.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe \"changed\" flag comes from refresh_cache_ent, which in turn gets it\nfrom ie_modified_stat. The one hesitation I have is that intent-to-add\nentries get the TYPE_CHANGED flag set, which means they will get a \"T\"\noutput. Whereas I actually think \"M\" is a little more sensible.\n\nFor \"git reset\", I'm not sure if it matters, since resetting the index\nwill always drop such an entry anyway. But you can see it with:\n\n  git add -N file\n  git add --refresh -v other\n\n read-cache.c |    6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 83fb19c..0e17add 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1107,14 +1107,17 @@ 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 *modified_fmt;\n \tconst char *deleted_fmt;\n+\tconst char *typechange_fmt;\n \tconst char *unmerged_fmt;\n \n \tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n \tdeleted_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n+\ttypechange_fmt = (in_porcelain ? \"T\\t%s\\n\" : \"%s needs update\\n\");\n \tunmerged_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+\t\tint changed = 0;\n \n \t\tce = istate->cache[i];\n \t\tif (ignore_submodules && S_ISGITLINK(ce->ce_mode))\n@@ -1135,7 +1138,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\tif (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n \t\t\tcontinue;\n \n-\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);\n+\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);\n \t\tif (new == ce)\n \t\t\tcontinue;\n \t\tif (!new) {\n@@ -1151,6 +1154,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\tif (quiet)\n \t\t\t\tcontinue;\n \t\t\tshow_file((cache_errno == ENOENT ? deleted_fmt :\n+\t\t\t\t   changed & TYPE_CHANGED ? typechange_fmt :\n \t\t\t\t   modified_fmt),\n \t\t\t\t  ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179461","messageId":"7vaa7yi6wv.fsf@alter.siamese.dyndns.org","threadId":"28869","inReplyTo":"20111114225651.GD3993@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] refresh_index: notice typechanges in output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-15T00:08:32Z","receivedAt":"2011-11-15T00:08:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> If a file changes type and a porcelain updates the index, we\n> will print \"M file\". Instead, let's be more specific and\n> print \"T file\", which matches actual diff and status output.\n>\n> The plumbing version remains \"needs update\" for historical\n> compatibility.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> The \"changed\" flag comes from refresh_cache_ent, which in turn gets it\n> from ie_modified_stat. The one hesitation I have is that intent-to-add\n> entries get the TYPE_CHANGED flag set, which means they will get a \"T\"\n> output. Whereas I actually think \"M\" is a little more sensible.\n\nI agree that we should not say that an intent-to-add entry has changed\ntype relative to whatever, as by definition there is nothing to compare\nagainst. \"A\" that stands for \"A\"dd is a lot more sensible here, I would\nthink.\n\n read-cache.c |   17 ++++++++++++++---\n 1 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 0e17add..27e9fc6 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1108,11 +1108,13 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tconst char *modified_fmt;\n \tconst char *deleted_fmt;\n \tconst char *typechange_fmt;\n+\tconst char *added_fmt;\n \tconst char *unmerged_fmt;\n \n \tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n \tdeleted_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n \ttypechange_fmt = (in_porcelain ? \"T\\t%s\\n\" : \"%s needs update\\n\");\n+\tadded_fmt = (in_porcelain ? \"A\\t%s\\n\" : \"%s needs update\\n\");\n \tunmerged_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@@ -1142,6 +1144,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\tif (new == ce)\n \t\t\tcontinue;\n \t\tif (!new) {\n+\t\t\tconst char *fmt;\n+\n \t\t\tif (not_new && cache_errno == ENOENT)\n \t\t\t\tcontinue;\n \t\t\tif (really && cache_errno == EINVAL) {\n@@ -1153,9 +1157,16 @@ 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\tshow_file((cache_errno == ENOENT ? deleted_fmt :\n-\t\t\t\t   changed & TYPE_CHANGED ? typechange_fmt :\n-\t\t\t\t   modified_fmt),\n+\n+\t\t\tif (cache_errno == ENOENT)\n+\t\t\t\tfmt = deleted_fmt;\n+\t\t\telse if (ce->ce_flags & CE_INTENT_TO_ADD)\n+\t\t\t\tfmt = added_fmt; /* must be before other checks */\n+\t\t\telse if (changed & TYPE_CHANGED)\n+\t\t\t\tfmt = typechange_fmt;\n+\t\t\telse\n+\t\t\t\tfmt = modified_fmt;\n+\t\t\tshow_file(fmt,\n \t\t\t\t  ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n"},{"id":"179463","messageId":"20111115020506.GA6305@sigill.intra.peff.net","threadId":"28869","inReplyTo":"7vaa7yi6wv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] refresh_index: notice typechanges in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-15T02:05:06Z","receivedAt":"2011-11-15T02:05:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 14, 2011 at 04:08:32PM -0800, Junio C Hamano wrote:\n\n> I agree that we should not say that an intent-to-add entry has changed\n> type relative to whatever, as by definition there is nothing to compare\n> against. \"A\" that stands for \"A\"dd is a lot more sensible here, I would\n> think.\n\nYeah, that makes sense to me.\n\n> +\t\t\tif (cache_errno == ENOENT)\n> +\t\t\t\tfmt = deleted_fmt;\n> +\t\t\telse if (ce->ce_flags & CE_INTENT_TO_ADD)\n> +\t\t\t\tfmt = added_fmt; /* must be before other checks */\n\nThanks, I was trying to figure out how to tell an intent-to-add file\nfrom the 'changed' flag, but obviously looking for the bit in the cache\nentry is the right thing.\n\nDo you want to add your patch on top, or do you want me to re-roll with\nthis squashed in? I can also hold the re-roll until post-release if you\nwant.\n\n-Peff\n"},{"id":"179689","messageId":"20111118110938.GA5940@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111115020506.GA6305@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] refresh_index: notice typechanges in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-18T11:09:38Z","receivedAt":"2011-11-18T11:09:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 14, 2011 at 09:05:06PM -0500, Jeff King wrote:\n\n> Do you want to add your patch on top, or do you want me to re-roll with\n> this squashed in? I can also hold the re-roll until post-release if you\n> want.\n\nYou mentioned squashing in the \"what's cooking\" message. Rather than\nsquashing just the typechange bits, how about this re-roll, which I\nthink is a little easier to follow:\n\n  [1/3]: read-cache: let refresh_cache_ent pass up changed flags\n  [2/3]: refresh_index: rename format variables\n  [3/3]: refresh_index: make porcelain output more specific\n\n-Peff\n"},{"id":"179690","messageId":"20111118111108.GA6335@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111118110938.GA5940@sigill.intra.peff.net","subject":"[PATCHv2 1/3] read-cache: let refresh_cache_ent pass up changed flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-18T11:11:08Z","receivedAt":"2011-11-18T11:11:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This will enable refresh_cache to differentiate more cases\nof modification (such as typechange) when telling the user\nwhat isn't fresh.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSame as 3/4 from my v1 series.\n\n read-cache.c |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 5790a91..8e69ea3 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1001,7 +1001,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n  */\n static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \t\t\t\t\t     struct cache_entry *ce,\n-\t\t\t\t\t     unsigned int options, int *err)\n+\t\t\t\t\t     unsigned int options, int *err,\n+\t\t\t\t\t     int *changed_ret)\n {\n \tstruct stat st;\n \tstruct cache_entry *updated;\n@@ -1033,6 +1034,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n \t}\n \n \tchanged = ie_match_stat(istate, ce, &st, options);\n+\tif (changed_ret)\n+\t\t*changed_ret = changed;\n \tif (!changed) {\n \t\t/*\n \t\t * The path is unchanged.  If we were told to ignore\n@@ -1130,7 +1133,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\tif (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n \t\t\tcontinue;\n \n-\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno);\n+\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);\n \t\tif (new == ce)\n \t\t\tcontinue;\n \t\tif (!new) {\n@@ -1157,7 +1160,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)\n {\n-\treturn refresh_cache_ent(&the_index, ce, really, NULL);\n+\treturn refresh_cache_ent(&the_index, ce, really, NULL, NULL);\n }\n \n static int verify_hdr(struct cache_header *hdr, unsigned long size)\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179691","messageId":"20111118111128.GB6335@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111118110938.GA5940@sigill.intra.peff.net","subject":"[PATCHv2 2/3] refresh_index: rename format variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-18T11:11:28Z","receivedAt":"2011-11-18T11:11:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When refreshing the index, for modified (or unmerged) files\nwe will print \"needs update\" (or \"needs merge\") for plumbing,\nor a \"diff --name-status\"-ish line for porcelain.\n\nThe variables holding which type of message to show are\nnamed after the plumbing messages. However, as we begin to\ndifferentiate more cases at the porcelain level (with the\nplumbing message stayin the same), that naming scheme will\nbecome awkward.\n\nInstead, name the variables after which case we found\n(modified or unmerged), not what we will output.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSame as 1/4 from my v1 series.\n\n read-cache.c |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 8e69ea3..f19b2f1 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1105,11 +1105,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\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_fmt;\n-\tconst char *needs_merge_fmt;\n+\tconst char *modified_fmt;\n+\tconst char *unmerged_fmt;\n \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+\tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tunmerged_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@@ -1125,7 +1125,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\tshow_file(needs_merge_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\tshow_file(unmerged_fmt, ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n@@ -1148,7 +1148,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\tshow_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);\n+\t\t\tshow_file(modified_fmt, ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179692","messageId":"20111118111308.GC6335@sigill.intra.peff.net","threadId":"28869","inReplyTo":"20111118110938.GA5940@sigill.intra.peff.net","subject":"[PATCH 3/3] refresh_index: make porcelain output more specific","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-11-18T11:13:08Z","receivedAt":"2011-11-18T11:13:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If you have a deleted file and a porcelain refreshes the\ncache, we print:\n\n  Unstaged changes after reset:\n  M\tfile\n\nThis is technically correct, in that the file is modified,\nbut it's friendlier to the user if we further differentiate\nthe case of a deleted file (especially because this output\nlooks a lot like \"diff --name-status\", which would also make\nthe distinction).\n\nSimilarly, we can distinguish typechanges (\"T\") and\nintent-to-add files (\"A\"), both of which appear as just \"M\"\nin the current output.\n\nThe plumbing output for all cases remains \"needs update\" for\nhistorical compatibility.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis is my deletion and typechange patches squashed together with your\nintent-to-add, and using an if/else series instead of the ternary\noperator, as you did.\n\n read-cache.c |   23 +++++++++++++++++++++--\n 1 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex f19b2f1..27e9fc6 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1106,13 +1106,20 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;\n \tconst char *modified_fmt;\n+\tconst char *deleted_fmt;\n+\tconst char *typechange_fmt;\n+\tconst char *added_fmt;\n \tconst char *unmerged_fmt;\n \n \tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n+\tdeleted_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n+\ttypechange_fmt = (in_porcelain ? \"T\\t%s\\n\" : \"%s needs update\\n\");\n+\tadded_fmt = (in_porcelain ? \"A\\t%s\\n\" : \"%s needs update\\n\");\n \tunmerged_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+\t\tint changed = 0;\n \n \t\tce = istate->cache[i];\n \t\tif (ignore_submodules && S_ISGITLINK(ce->ce_mode))\n@@ -1133,10 +1140,12 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\tif (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))\n \t\t\tcontinue;\n \n-\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, NULL);\n+\t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);\n \t\tif (new == ce)\n \t\t\tcontinue;\n \t\tif (!new) {\n+\t\t\tconst char *fmt;\n+\n \t\t\tif (not_new && cache_errno == ENOENT)\n \t\t\t\tcontinue;\n \t\t\tif (really && cache_errno == EINVAL) {\n@@ -1148,7 +1157,17 @@ 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\tshow_file(modified_fmt, ce->name, in_porcelain, &first, header_msg);\n+\n+\t\t\tif (cache_errno == ENOENT)\n+\t\t\t\tfmt = deleted_fmt;\n+\t\t\telse if (ce->ce_flags & CE_INTENT_TO_ADD)\n+\t\t\t\tfmt = added_fmt; /* must be before other checks */\n+\t\t\telse if (changed & TYPE_CHANGED)\n+\t\t\t\tfmt = typechange_fmt;\n+\t\t\telse\n+\t\t\t\tfmt = modified_fmt;\n+\t\t\tshow_file(fmt,\n+\t\t\t\t  ce->name, in_porcelain, &first, header_msg);\n \t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n-- \n1.7.7.3.8.g38efa\n"},{"id":"179717","messageId":"7vwraxjhaa.fsf@alter.siamese.dyndns.org","threadId":"28869","inReplyTo":"20111118110938.GA5940@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] refresh_index: notice typechanges in output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-18T20:40:29Z","receivedAt":"2011-11-18T20:40:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Nov 14, 2011 at 09:05:06PM -0500, Jeff King wrote:\n>\n>> Do you want to add your patch on top, or do you want me to re-roll with\n>> this squashed in? I can also hold the re-roll until post-release if you\n>> want.\n>\n> You mentioned squashing in the \"what's cooking\" message. Rather than\n> squashing just the typechange bits, how about this re-roll, which I\n> think is a little easier to follow:\n>\n>   [1/3]: read-cache: let refresh_cache_ent pass up changed flags\n>   [2/3]: refresh_index: rename format variables\n>   [3/3]: refresh_index: make porcelain output more specific\n\nLooks sensible; thanks. Will replace.\n"}]}