[PATCH] sortdict: fix .pop() to return a value

[David Soria Parra]

On Sun, Apr 09, 2017 at 10:10:01PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at 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