Submitter | phabricator |
---|---|
Date | April 2, 2019, 7:30 p.m. |
Message ID | <differential-rev-PHID-DREV-tjcsdesv36puuzefzclz-req@phab.mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/39446/ |
State | Superseded |
Headers | show |
Comments
> +def decodeextra(text, default): > """ > >>> from .pycompat import bytechr as chr > - >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}) > - ... ).items()) > + >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}), > + ... _defaultextra).items()) > [('baz', '\\x002'), ('branch', 'default'), ('foo', 'bar')] > >>> sorted(decodeextra(encodeextra({b'foo': b'bar', > - ... b'baz': chr(92) + chr(0) + b'2'}) > - ... ).items()) > + ... b'baz': chr(92) + chr(0) + b'2'}), > + ... _defaultextra).items()) Maybe these tests update `_defaultextra` in place? > [('baz', '\\\\\\x002'), ('branch', 'default'), ('foo', 'bar')] > """ > - extra = _defaultextra.copy() > + extra = default I think it's safer to do `default.copy()` here. > for l in text.split('\0'): > if l: > k, v = _string_unescape(l).split(':', 1) > @@ -275,7 +275,7 @@ > if raw is None: > return _defaultextra > > - return decodeextra(raw) > + return decodeextra(raw, _defaultextra.copy())
yuja added a comment. > +def decodeextra(text, default): > > """ > >>> from .pycompat import bytechr as chr > > - >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}) > - ... ).items()) + >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}), + ... _defaultextra).items()) [('baz', '\\x002'), ('branch', 'default'), ('foo', 'bar')] >>> sorted(decodeextra(encodeextra({b'foo': b'bar', > - ... b'baz': chr(92) + chr(0) + b'2'}) > - ... ).items()) + ... b'baz': chr(92) + chr(0) + b'2'}), + ... _defaultextra).items()) Maybe these tests update `_defaultextra` in place? > [('baz', '\\\\\\x002'), ('branch', 'default'), ('foo', 'bar')] > """ > > - extra = _defaultextra.copy() + extra = default I think it's safer to do `default.copy()` here. > for l in text.split('\0'): > if l: > k, v = _string_unescape(l).split(':', 1) > > @@ -275,7 +275,7 @@ > > if raw is None: > return _defaultextra > > > - return decodeextra(raw) + return decodeextra(raw, _defaultextra.copy()) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6185 To: martinvonz, #hg-reviewers Cc: yuja, mercurial-devel
martinvonz added a comment. In https://phab.mercurial-scm.org/D6185#90696, @yuja wrote: > > +def decodeextra(text, default): > > > > """ > > >>> from .pycompat import bytechr as chr > > > > - >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}) > > - ... ).items()) + >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}), + ... _defaultextra).items()) [('baz', '\\x002'), ('branch', 'default'), ('foo', 'bar')] >>> sorted(decodeextra(encodeextra({b'foo': b'bar', > > - ... b'baz': chr(92) + chr(0) + b'2'}) > > - ... ).items()) + ... b'baz': chr(92) + chr(0) + b'2'}), + ... _defaultextra).items()) > > Maybe these tests update `_defaultextra` in place? > > > [('baz', '\\\\\\x002'), ('branch', 'default'), ('foo', 'bar')] > > """ > > > > - extra = _defaultextra.copy() + extra = default > > I think it's safer to do `default.copy()` here. Good point. I'll change it. > > >> for l in text.split('\0'): >> if l: >> k, v = _string_unescape(l).split(':', 1) >> >> @@ -275,7 +275,7 @@ >> >> if raw is None: >> return _defaultextra >> >> >> - return decodeextra(raw) + return decodeextra(raw, _defaultextra.copy()) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6185 To: martinvonz, #hg-reviewers Cc: yuja, mercurial-devel
martinvonz abandoned this revision. martinvonz added a comment. Oh, I actually don't need this patch anymore since I added the string_unescape() method. I'll drop this one. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6185 To: martinvonz, #hg-reviewers Cc: yuja, mercurial-devel
Patch
diff --git a/mercurial/changelog.py b/mercurial/changelog.py --- a/mercurial/changelog.py +++ b/mercurial/changelog.py @@ -54,18 +54,18 @@ text = text.replace('\n', '') return stringutil.unescapestr(text) -def decodeextra(text): +def decodeextra(text, default): """ >>> from .pycompat import bytechr as chr - >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}) - ... ).items()) + >>> sorted(decodeextra(encodeextra({b'foo': b'bar', b'baz': chr(0) + b'2'}), + ... _defaultextra).items()) [('baz', '\\x002'), ('branch', 'default'), ('foo', 'bar')] >>> sorted(decodeextra(encodeextra({b'foo': b'bar', - ... b'baz': chr(92) + chr(0) + b'2'}) - ... ).items()) + ... b'baz': chr(92) + chr(0) + b'2'}), + ... _defaultextra).items()) [('baz', '\\\\\\x002'), ('branch', 'default'), ('foo', 'bar')] """ - extra = _defaultextra.copy() + extra = default for l in text.split('\0'): if l: k, v = _string_unescape(l).split(':', 1) @@ -275,7 +275,7 @@ if raw is None: return _defaultextra - return decodeextra(raw) + return decodeextra(raw, _defaultextra.copy()) @property def files(self):