Patchwork [1,of,8,RFC] localrepo: extend "changeid in repo" to return True for workingctx revision

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 19, 2014, 10:56 p.m.
Message ID <0e8b104fd419419ddf9e.1408488975@mimosa>
Download mbox | patch
Permalink /patch/5524/
State Superseded
Commit b9f7f3eeb99c206897beacc0eb323ab9c178e599
Headers show

Comments

Yuya Nishihara - Aug. 19, 2014, 10:56 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1408242931 -32400
#      Sun Aug 17 11:35:31 2014 +0900
# Node ID 0e8b104fd419419ddf9ec3c9df4df1991e9ba3a5
# Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
localrepo: extend "changeid in repo" to return True for workingctx revision

This is necessary to implement revision specifier for workingctx, that will
be used like

    $ hg annotate -r workingdir FILE

In principle, "rev in repo" should be True if "repo[rev]" can return context
object.  But when it was implemented by ea3acaae25bb, lookup() had long logic
to map all sorts of changeid to node, and "None in repo" did crash because
lookup() did not support it.  So I guess the case of changeid=None was not
considered.

Since now "None in repo" doesn't crash, it should be True for workingctx
revision.

Behavior of "changeid in repo":

     X in repo  "null"  existing rev  None (workingctx)
    ----------  ------  ------------  -----------------
    original*   True    True          TypeError
    current     True    True          False
    this patch  True    True          True

    (*original: ea3acaae25bb)
Pierre-Yves David - Aug. 19, 2014, 11:41 p.m.
On 08/19/2014 03:56 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1408242931 -32400
> #      Sun Aug 17 11:35:31 2014 +0900
> # Node ID 0e8b104fd419419ddf9ec3c9df4df1991e9ba3a5
> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
> localrepo: extend "changeid in repo" to return True for workingctx revision

I love the idea of having annotate agains working directory (could 
probably be extended to some other command) but I'm really not a fan of 
messing with the revision rumbering.

Is there a way to get the same feature without messaging with a 9 yeah 
old numerotation scheme?

>
> This is necessary to implement revision specifier for workingctx, that will
> be used like
>
>      $ hg annotate -r workingdir FILE
>
> In principle, "rev in repo" should be True if "repo[rev]" can return context
> object.  But when it was implemented by ea3acaae25bb, lookup() had long logic
> to map all sorts of changeid to node, and "None in repo" did crash because
> lookup() did not support it.  So I guess the case of changeid=None was not
> considered.
>
> Since now "None in repo" doesn't crash, it should be True for workingctx
> revision.
>
> Behavior of "changeid in repo":
>
>       X in repo  "null"  existing rev  None (workingctx)
>      ----------  ------  ------------  -----------------
>      original*   True    True          TypeError
>      current     True    True          False
>      this patch  True    True          True
>
>      (*original: ea3acaae25bb)
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -449,7 +449,8 @@ class localrepository(object):
>
>       def __contains__(self, changeid):
>           try:
> -            return bool(self.lookup(changeid))
> +            self[changeid]
> +            return True
>           except error.RepoLookupError:
>               return False
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - Aug. 20, 2014, 3:01 p.m.
On Tue, 19 Aug 2014 16:41:29 -0700, Pierre-Yves David wrote:
> On 08/19/2014 03:56 PM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1408242931 -32400
> > #      Sun Aug 17 11:35:31 2014 +0900
> > # Node ID 0e8b104fd419419ddf9ec3c9df4df1991e9ba3a5
> > # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
> > localrepo: extend "changeid in repo" to return True for workingctx revision
> 
> I love the idea of having annotate agains working directory (could 
> probably be extended to some other command) but I'm really not a fan of 
> messing with the revision rumbering.

I see.

> Is there a way to get the same feature without messaging with a 9 yeah 
> old numerotation scheme?

Maybe the easiest way is to add -W,--working-directory option.  It won't
break existing code base.

Regards,
Pierre-Yves David - Aug. 21, 2014, 5:03 p.m.
On 08/20/2014 08:01 AM, Yuya Nishihara wrote:
> On Tue, 19 Aug 2014 16:41:29 -0700, Pierre-Yves David wrote:
>> On 08/19/2014 03:56 PM, Yuya Nishihara wrote:
>>> # HG changeset patch
>>> # User Yuya Nishihara <yuya@tcha.org>
>>> # Date 1408242931 -32400
>>> #      Sun Aug 17 11:35:31 2014 +0900
>>> # Node ID 0e8b104fd419419ddf9ec3c9df4df1991e9ba3a5
>>> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
>>> localrepo: extend "changeid in repo" to return True for workingctx revision
>>
>> I love the idea of having annotate agains working directory (could
>> probably be extended to some other command) but I'm really not a fan of
>> messing with the revision rumbering.
>
> I see.
>
>> Is there a way to get the same feature without messaging with a 9 yeah
>> old numerotation scheme?
>
> Maybe the easiest way is to add -W,--working-directory option.  It won't
> break existing code base.

The idea of being able to use the working directory sounds good. Can we 
make a list of command where it would make sense? (and a list of where 
it would be actually useful). If the corpus is big enough, it probably 
worth implementing something in core.

I made a deeper review of your series, and you seems to "confuse" 
commitablectx with working directory. Commitablectx may also be in 
memctx (in memory changeset about to be commited) it is not clear if you 
took that in account in your patches. That said, being able to run 
operation on in memory ctx may be quite nice too (no precise usecase yet)

Conclusion: the implementation is too bold, but the idea might worth 
pursuing.
Yuya Nishihara - Aug. 22, 2014, 3:17 p.m.
On Thu, 21 Aug 2014 10:03:43 -0700, Pierre-Yves David wrote:
> On 08/20/2014 08:01 AM, Yuya Nishihara wrote:
> > On Tue, 19 Aug 2014 16:41:29 -0700, Pierre-Yves David wrote:
> >> On 08/19/2014 03:56 PM, Yuya Nishihara wrote:
> >>> # HG changeset patch
> >>> # User Yuya Nishihara <yuya@tcha.org>
> >>> # Date 1408242931 -32400
> >>> #      Sun Aug 17 11:35:31 2014 +0900
> >>> # Node ID 0e8b104fd419419ddf9ec3c9df4df1991e9ba3a5
> >>> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
> >>> localrepo: extend "changeid in repo" to return True for workingctx revision
> >>
> >> I love the idea of having annotate agains working directory (could
> >> probably be extended to some other command) but I'm really not a fan of
> >> messing with the revision rumbering.
> >
> > I see.
> >
> >> Is there a way to get the same feature without messaging with a 9 yeah
> >> old numerotation scheme?
> >
> > Maybe the easiest way is to add -W,--working-directory option.  It won't
> > break existing code base.
> 
> The idea of being able to use the working directory sounds good. Can we 
> make a list of command where it would make sense? (and a list of where 
> it would be actually useful). If the corpus is big enough, it probably 
> worth implementing something in core.

This is the short list of core commands that can (or already) support
working-directory revision.

    command   default  is workingdir option or revision useful?
    --------  -------  -------------------------------------------------------
    annotate  p1       useful
    archive   p1       might be useful on Windows (no tar or zip)
    cat       p1       might be useful on Windows (no cat)
    diff      p1:+     (default)
    export    p1       might be usable if wctx (or memctx?) can have pre-edit
                       commit message
    grep      tip:0    might be useful on Windows (no grep)
    identify  +        (default)
    locate    +        (default)
    log       tip:0    might be usable with -p or -G option (but it would
                       complicate the implementation)
    parents   +        (default)
    status    +        (default)

    (+: working-directory revision)

> I made a deeper review of your series, and you seems to "confuse" 
> commitablectx with working directory. Commitablectx may also be in 
> memctx (in memory changeset about to be commited) it is not clear if you 
> took that in account in your patches. That said, being able to run 
> operation on in memory ctx may be quite nice too (no precise usecase yet)
>
> Conclusion: the implementation is too bold, but the idea might worth 
> pursuing.

Right.  In this patch series, I didn't consider the "committable" aspect of
workingctx and memctx.

Regards,

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -449,7 +449,8 @@  class localrepository(object):
 
     def __contains__(self, changeid):
         try:
-            return bool(self.lookup(changeid))
+            self[changeid]
+            return True
         except error.RepoLookupError:
             return False