Re: reset reports file as modified when it's in fact deleted

On Fri, Nov 11, 2011 at 08:43:34AM -0800, Junio C Hamano wrote:

Carlos Martín Nieto [off-list ref] writes:

quoted

On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:
...

quoted

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