[PATCH 0 of 5] remove

Matt Mackall mpm at selenic.com
Mon May 23 15:22:39 CDT 2011


On Mon, 2011-05-23 at 22:01 +0200, Adrian Buehlmann wrote:
> On 2011-05-23 21:48, Matt Mackall wrote:
> > On Mon, 2011-05-23 at 21:33 +0200, Adrian Buehlmann wrote:
> >> On 2011-05-23 21:26, Matt Mackall wrote:
> >>> On Mon, 2011-05-23 at 21:07 +0200, Adrian Buehlmann wrote:
> >>>> On 2011-05-23 20:25, Matt Mackall wrote:
> >>>>> On Mon, 2011-05-23 at 16:56 +0200, Adrian Buehlmann wrote:
> >>>>>> see patches
> >>>>>
> >>>>> I'm afraid I can't really see what's motivating this, can you elucidate
> >>>>> please?
> >>>>>
> >>>>
> >>>> If you don't want to look at the patches: No.
> >>>
> >>> I did look, was confused, asked for clarification, and got rude
> >>> response. If you'd rather that I drop your patches than spend a couple
> >>> sentences explaining why we should want them, then I suppose that's your
> >>> choice. Moving on..
> >>>
> >>
> >> All I tried to say is, that you have to look at the individual patches.
> >> I don't think I was rude?
> > 
> > Is this a collection of unrelated patches? It appears not, but I can't
> > quite work out what the goal is. There's some stuff here that appears
> > iffy to me, knowing what the larger goal is will help me evaluate it.
> > 
> 
> Patch 2 depends on patch 1.
> 
> Can you look at patch 2 and see if that makes any sense for you? I wrote
> a comment to that.

Ok, so my sense now is that this is recapitulating the oscillation we've
had with remove/forget at the command layer, except at the mid-layer.

Ages ago, we had remove and forget commands. Then some eager coder
decided that forget was a special case of remove and folded it into
remove as a flag. And then we had to grow that really confusing behavior
table as the behavior now wasn't obvious to everyone (or anyone,
actually). The behavior of remove is sensible on inspection, but no one
can be expected to arrive at it from first principles.

And then we decided that this was all way more pain than it was worth
and added the forget command back as the obvious way to undo an add.

This seems to be the start of a similar undertaking, except internal. We
have remove and forget at the command layer (with remove having some
forget magic), remove and forget at the context layer, and remove and
forget at the dirstate layer. I don't see that dropping forget in the
mid-layer in exchange for a second argument to ctx.remove is a win.

(Though it is a little suspect that commands.forget is calling
ctx.remove!)

> Patch 3 removes function forget of workingctx, which is obsoleted by
> patches 1 and 2.
> 
> Patch 4 is not strictly related, but I happened to have that done after
> 3. As I wrote in the comment there: it moves the unlinking of the files
> to under the lock.

That seems sensible. I'll definitely take that one if you send it
separately.

> Patch 5 is not particularly related, it just improves the help text if
> of the 'hg remove' command.

And this one is of course fine.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list