On Sun, Apr 09, 2017 at 10:10:01PM +0900, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <y...@tcha.org> > # Date 1491706629 -32400 > # Sun Apr 09 11:57:09 2017 +0900 > # Node ID 48a7a1f77a9489e3c4b5f862243782ceae80eaf9 > # Parent 9259cf823690e4fcd34a4d2ecd57ced2060d2b3d > sortdict: fix .pop() to return a value > > My future patch will need it.
Generally this looks fine to me. > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -555,11 +555,11 @@ class sortdict(dict): > dict.__delitem__(self, key) > self._list.remove(key) > def pop(self, key, *args, **kwargs): > - dict.pop(self, key, *args, **kwargs) > try: > self._list.remove(key) > except ValueError: > pass > + return dict.pop(self, key, *args, **kwargs) Note that this breaks the insert/delete asymmetry. We usually insert into the _list first and then add it to the dictionary and the current version of pop deletes from the dictionary first and then from the list ensuring an add to list add to dict del from dict del from list order. Your patch breaks this. I wonder if can lead to unwanted behavior. If we want to be on the save side we should probably do res = dict.pop(...) try: self._list.remove() ... return res _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel