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

From: Jeff King <peff@peff.net>
Date: Nov 11, 2011, 18:21 UTC
Message-ID: <20111111182109.GB16055@sigill.intra.peff.net>
In-Reply-To: <7vr51er4nd.fsf@alter.siamese.dyndns.org>

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

Show 30 quoted lines

> Carlos Martín Nieto <cmn@elego.de> 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