Patchwork D6185: changelog: pass default extras into decodeextra()

login
register
mail settings
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

phabricator - April 2, 2019, 7:30 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I want to use the function with a different default (empty dict, to be
  specific) and this enables that.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6185

AFFECTED FILES
  mercurial/changelog.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - April 13, 2019, 11:36 a.m.
> +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())
phabricator - April 13, 2019, 11:45 a.m.
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
phabricator - April 13, 2019, 10:14 p.m.
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
phabricator - April 13, 2019, 10:19 p.m.
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):