[PATCH 1 of 6] sortdict: have update() accept either dict or iterable of key/value pairs
Ryan McElroy
rm at fb.com
Mon Mar 9 02:05:00 CDT 2015
On 3/8/2015 4:56 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1424267633 -32400
> # Wed Feb 18 22:53:53 2015 +0900
> # Node ID 023d0f46318665e8d01fe42fb58ac1726532b4c0
> # Parent 62c4a963489d0ff8887b1e5d2c9458d1e3384536
> sortdict: have update() accept either dict or iterable of key/value pairs
I'm always skeptical about making APIs accept more things -- it makes
the code harder to understand and change in the future. I'd prefer that
all callers of this function be changed to pass key-value pair
iterables. I understand that might be hard though (because typehints
don't exist, for one thing), so I'll leave it up to those who know more
to decide if you should listen to this objection of mine or not :-)
>
> Future patches will make templater stores sorted dict in _hybrid object.
> sortdict should be constructed from sorted list.
English grammar nit-picking (changes in bold): "Future patches will make
*the *templater *store **a **sortdict *in *the *_hybrid object. *This
*sortdict should be constructed from *a *sorted list."
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -359,8 +359,10 @@ class sortdict(dict):
> def __iter__(self):
> return self._list.__iter__()
> def update(self, src):
> - for k in src:
> - self[k] = src[k]
> + if isinstance(src, dict):
> + src = src.iteritems()
> + for k, v in src:
> + self[k] = v
> def clear(self):
> dict.clear(self)
> self._list = []
>
Contents of patch look fine, despite my objections to the premise.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150309/e8c6c19a/attachment.html>
More information about the Mercurial-devel
mailing list