On Fri, Nov 11, 2011 at 08:43:34AM -0800, Junio C Hamano wrote: > Carlos Martín Nieto <cmn@xxxxxxxx> writes: > > > On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote: > > ... > >> The patch to do "D"eleted is pretty simple: > >> > >> diff --git a/read-cache.c b/read-cache.c > >> index dea7cd8..cc1ebdf 100644 > >> --- a/read-cache.c > >> +++ b/read-cache.c > >> @@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p > >> int in_porcelain = (flags & REFRESH_IN_PORCELAIN); > >> unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0; > >> const char *needs_update_fmt; > >> + const char *needs_rm_fmt; > >> const char *needs_merge_fmt; > >> > >> needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n"); > >> + needs_rm_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n"); > >> needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n"); > > > > While the name fits in with the rest of the variables, it's kind of > > the wrong way around, isn't it? It doesn't need an 'rm', it /was/ > > rm'd. > > The variable names were chosen to mean "In a situation where the plumbing > traditionally would have said X, use this format to describe it". This is > the first topic to separate a single situation (from the plumbing's point > of view) into two and say different things at Porcelain, and the variable > naming no longer works. I agree the naming is awkward (but then, I think the "needs update" message is awkward ;) ). But my interpretation was: if you want the index to be in sync with the working tree, you must do this. Hence: $EDITOR file ;# triggers needs_update git update-index file ;# and do update in index rm file ;# triggers needs rm git rm --cached file ;# and do rm in index So that was my attempt to follow the same scheme, and I think it does work. But I agree it's awkward, and since we're not changing the plumbing message (nor do I think we should; most users won't see it, and we would only be breaking scripts that do), it's not a big deal just to change the variable names. > An obvious solution would be to rename all of them to be based on "what > happened to the path". E.g. "modified_fmt" would be set to either "M" or > "needs update", and "removed_fmt" would be set to either "D" or "needs > update", etc. I'm happy to make that change. What do you think of the feature overall? And should typechanges also be handled here (it's no extra work for git to detect them; we just have to pass the TYPE_CHANGED flag back up the stack)? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html