D2969: context: move reuse of context object to repo.__getitem__ (API)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGbb47dc2f71a0: context: move reuse of context object to repo.__getitem__ (API) (authored by martinvonz, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D2969?vs=7371=7392 REVISION DETAIL https://phab.mercurial-scm.org/D2969 AFFECTED FILES mercurial/context.py mercurial/localrepo.py CHANGE DETAILS diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -782,6 +782,8 @@ def __getitem__(self, changeid): if changeid is None: return context.workingctx(self) +if isinstance(changeid, context.basectx): +return changeid if isinstance(changeid, slice): # wdirrev isn't contiguous so the slice shouldn't include it return [context.changectx(self, i) diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -62,11 +62,6 @@ be committed, memctx: a context that represents changes in-memory and can also be committed.""" -def __new__(cls, repo, changeid='', *args, **kwargs): -if isinstance(changeid, basectx): -return changeid - -return super(basectx, cls).__new__(cls) def __bytes__(self): return short(self.node()) @@ -412,11 +407,6 @@ def __init__(self, repo, changeid='.'): """changeid is a revision number, node, or tag""" -# since basectx.__new__ already took care of copying the object, we -# don't need to do anything in __init__, so we just exit here -if isinstance(changeid, basectx): -return - if changeid == '': changeid = '.' self._repo = repo @@ -2486,9 +2476,6 @@ dateutil.parsedate() and defaults to current date, extra is a dictionary of metadata or is left empty. """ -def __new__(cls, repo, originalctx, *args, **kwargs): -return super(metadataonlyctx, cls).__new__(cls, repo) - def __init__(self, repo, originalctx, parents=None, text=None, user=None, date=None, extra=None, editor=False): if text is None: To: martinvonz, #hg-reviewers, indygreg Cc: indygreg, yuja, smf, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2969: context: move reuse of context object to repo.__getitem__ (API)
indygreg accepted this revision. indygreg added a comment. This revision is now accepted and ready to land. That's 2 of us who think this is a good idea. I'm going to queue it. If @smf knows of a reason why this is bad, we can always drop it or revert it. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2969 To: martinvonz, #hg-reviewers, indygreg Cc: indygreg, yuja, smf, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2969: context: move reuse of context object to repo.__getitem__ (API)
yuja added subscribers: smf, yuja. yuja added a comment. +1, but I don't remember why we made that hack. @smf Any thoughts? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2969 To: martinvonz, #hg-reviewers Cc: yuja, smf, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2969: context: move reuse of context object to repo.__getitem__ (API)
martinvonz created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY As an example of how weird the basectx.__new__ is: whenever you create a workingctx, basectx.__new__ gets called first. Since our __new__ has a "changeid" argument as second parameter, when create the workingctx(repo, text="blah"), the text gets bound to "changeid". Since a string isn't a basectx, our __new__ ends up not doing anything funny, but that's still very confusing code. Another case is metadataonlyctx.__new__(), which I think exists in order to prevent metadataonlyctx.__init__'s third argument (originalctx) from being interpreted as a changeid in basectx.__new__(), thereby getting reused. Let's move this to repo.__getitem__ instead, where it will be pretty obvious what the code does. After this patch, changectx(ctx) will be an error (it will fail when trying to see if it's a 20-byte string). REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2969 AFFECTED FILES mercurial/context.py mercurial/localrepo.py CHANGE DETAILS diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -782,6 +782,8 @@ def __getitem__(self, changeid): if changeid is None: return context.workingctx(self) +if isinstance(changeid, context.basectx): +return changeid if isinstance(changeid, slice): # wdirrev isn't contiguous so the slice shouldn't include it return [context.changectx(self, i) diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -62,11 +62,6 @@ be committed, memctx: a context that represents changes in-memory and can also be committed.""" -def __new__(cls, repo, changeid='', *args, **kwargs): -if isinstance(changeid, basectx): -return changeid - -return super(basectx, cls).__new__(cls) def __bytes__(self): return short(self.node()) @@ -412,11 +407,6 @@ def __init__(self, repo, changeid='.'): """changeid is a revision number, node, or tag""" -# since basectx.__new__ already took care of copying the object, we -# don't need to do anything in __init__, so we just exit here -if isinstance(changeid, basectx): -return - if changeid == '': changeid = '.' self._repo = repo @@ -2486,9 +2476,6 @@ dateutil.parsedate() and defaults to current date, extra is a dictionary of metadata or is left empty. """ -def __new__(cls, repo, originalctx, *args, **kwargs): -return super(metadataonlyctx, cls).__new__(cls, repo) - def __init__(self, repo, originalctx, parents=None, text=None, user=None, date=None, extra=None, editor=False): if text is None: To: martinvonz, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel