[PATCH 1 of 1] commands.remove: don't use workingctx.remove(list, unlink=True)

Matt Mackall mpm at selenic.com
Thu May 26 14:59:36 CDT 2011


On Wed, 2011-05-25 at 22:21 +0200, Adrian Buehlmann wrote:
> On 2011-05-25 19:24, Matt Mackall wrote:
> > On Wed, 2011-05-25 at 14:45 +0200, Adrian Buehlmann wrote:
> >> # HG changeset patch
> >> # User Adrian Buehlmann <adrian at cadifra.com>
> >> # Date 1306313693 -7200
> >> # Node ID db3623327c9c02f44b6bd9374dfa8594b326820d
> >> # Parent  a6b543e053058c39b52e2b7c1e9a4b7c14c66a56
> >> commands.remove: don't use workingctx.remove(list, unlink=True)
> >>
> >> workingctx.remove(list, unlink=True) is unsuited here, because it does too
> >> much: it also unlinks added files.
> > 
> > I think we're converging here. I've done a survey of the behavior of the
> > remove/forget methods at all three levels. They all have significantly
> > different semantics.
> > 
> > W = warning
> > S = schedule removal from manifest (move to 'r' state)
> > D = delete from working directory
> > d = delete attempted
> > U = unlist from dirstate
> > - = silently ignore
> > 
> > A = added
> > C = clean
> > M = modified
> > ! = missing
> > ! = added but missing
> > ? = unknown
> > 
> > command level:
> > 
> >  remove: all sorts of things
> >          A  C  M  !  A! ?
> >   none   W  SD W  S  U  W  <- safe
> >   -f     U  SD SD S  U  W  <- dangerous
> >   -A     W  W  W  S  U  W  <- confusing
> >   -Af    U  S  S  S  U  W  <- same as forget, safe again
> > 
> >  forget: stop tracking file pattern
> >          A  C  M  !  A! ?
> >          U  S  S  S  U  W
> > 
> > context level:
> > 
> >  remove: stop tracking list of files, optional deletion
> >          A  C  M  !  A! ?
> >    none  U  S  S  S  U  W
> >  unlink  UD SD SD Sd Ud WD  <- we delete added/unknown files, then warn
> > 
> >  forget: stop tracking newly-added list of files
> >          A  C  M  !  A! ?
> >          U  W  W  W  W  W
> > 
> > dirstate level:
> >  remove: move a single file to the 'r' state
> >          A  C  M  !  A! ?
> >          S  S  S  S  S  -   <- moving 'a' to 'r' is nonsense
> >  forget: drop a single file 
> >          A  C  M  !  A! ?
> >          U  U  U  U  U  W   <- dropping 'n' state may be corrupt
> > 
> > Observations:
> > 
> > a) commands.forget is identical to context.remove(False)
> > b) context.remove(True) has some dangerous behavior
> > c) context.forget is very narrow
> > d) dirstate.remove and forget are overbroad
> > 
> > So it seems like we want to:
> > 
> > 1) move actual deletion out of context.remove() and drop the arg
> > 2) unify remove and forget into a single method at both context and
> > dirstate levels:
> > 
> >  newfunc:
> >          A  C  M  !  A! ?
> >          U  S  S  S  U  W
> > 
> > Note that if dirstate.newfunc knows how to deal with added files
> > specially, this has the advantage that context.newfunc can delegate that
> > detail and become very simple.
> > 
> > What this func should be called is not clear. On the one hand, it
> > precisely matches commands.forget behavior, so forget seems good. But it
> > also strongly disagrees with the existing context and dirstate.forget,
> > so there might be some short-term pain associated with that change.
> > 'remove' is not a good name because it doesn't agree with the command
> > layer. 'drop' is another possibility.
> 
> It's good that you have a plan.
> 
> Is there anything you are expecting from me that I say here?

Well, you could always tell me I'm wrong. Turns out I'm wrong here,
newfunc can't combine forget and remove at the dirstate level because we
sometimes need to explicitly drop tracked files.

> I think my patch conforms to your step 1), so it could be taken, because
> it conforms to your plan.

I have some patches moving in this direction now but I'm not sure about
the final result still.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list