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

Matt Mackall mpm at selenic.com
Wed May 25 12:24:21 CDT 2011

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


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:

         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.

Mathematics is the supreme nostalgia of our time.

More information about the Mercurial-devel mailing list