[PATCH] keyword: wrap repo.commitctx instead of commit

[Dan Villiom Podlaski Christiansen]

You might want to consider doing some sort of “double-wrapping”. That
is, add a wrapper for commit() which inserts or a wrapper of
commitctx(), or perhaps just enables it. This should allow you to
extend low-level operations whilst restricting the expansion to the
relevant user commands. A white list strikes me as something bound to
cause unexpected behaviour.

I've considered doing the same for hgsubversion, where we wrap the
clone command, but need the resulting repository objects. (We want to
add to the generated hgrc, but only when the source of the clone was a
Subversion repository.) The clone command itself doesn't return them,
so we currently call hg.clone() directly; it's a bit of a hack.
Wrapping both - using e.g. the repository instance to pass on
arguments - should be much cleaner.

On 30/06/2009, Christian Ebert <blacktrash at gmx.net> wrote:
> * Augie Fackler on Monday, June 29, 2009 at 21:01:35 -0500
>> On Jun 29, 2009, at 6:21 PM, Christian Ebert wrote:
>>> # HG changeset patch
>>> # User Christian Ebert <blacktrash at gmx.net>
>>> # Date 1246317449 -7200
>>> # Node ID 2a9b3d89cdda24eb72648a6e3b76c2cc4235c216
>>> # Parent  3d11f8337bf9956c1fceb7dd937e22053cf33386
>>> keyword: wrap repo.commitctx instead of commit
>>>
>>> This avoids forcing the dirstate of overwritten files
>>> to normal during a commit.
>>
>> How will this interact with other extensions that expect to use
>> commitctx directly? I'm personally concerned about convert, hg-git and
>> hgsubversion, which all use commitctx to produce their converted
>> revisions.
>
> Ah, right.
>
> convert poses no problem because keyword turns itself off at
> reposetup() while convert is run. However, previously (before the
> patch) it mainly did this for speed reasons (working dir not
> updated during convert).
>
> For testing I removed this switch and then:
>
> $ hg convert cvsrepo cvsrepo-hg
>
> same as before because keyword returns immediately if not
> repo.local().
>
> But
>
> $ hg convert ../cvsrepo .
>
> writes to the working directory during convert (files containing
> keywords to expand).
>
>> Should such extensions start checking for the presence of the keyword
>> wrapper and refuse to work if it is enabled?
>
> Not sure whether this can be detected reliably (load order of
> extensions etc.). But I guess the behaviour would be similar to
> convert when convert is turned on during conversion.
>
> What a shame. It looked like the fitting place for the wrapper
> after commit became more context centered. And I really like the
> fact that at least commit doesn't fiddle with the dirstate
> anymore (kwexpand still does, and probably has to by nature).
>
> Alternatives I see at the moment:
>
> 1) keyword could take care of this by adding the respective user
>    commands to keyword.nokwcommands.
>
>    Of course there might be other extensions out there that wrap
>    commitctx.
>
>    otoh there might be extensions that wrap repo.commit ...
>
> 2) It would be nice if extensions could turn keyword off.
>
>    Some time ago I tried and failed for "hg log" in tortoisehg.
>    And failed; probably because of the load order problem --
>    keyword.reposetup might already have run (and because I didn't
>    have patience to grasp what tortoishg actually does).
>
>    Another way out would be a repo.ui attribute set by any
>    extension that wants to set it, which keyword checks in each
>    subclassed repo command; for commitctx:
>
>    if ui.kwstop:
>        return super(kwrepo, self).commitctx(ctx, error)
>
> 3) leave everyting as is.
>
> c
> --
>   Was heißt hier Dogma, ich bin Underdogma!
> [ What the hell do you mean dogma, I am underdogma. ]
> _F R E E_  _V I D E O S_  http://www.blacktrash.org/underdogma/
>                           http://www.blacktrash.org/underdogma/index-en.html
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>


-- 
Dan Villiom Podlaski Christiansen, stud.scient.
danchr at gmail.com