Patchwork [01,of,10,py3] help: convert flag default to bytes portably

login
register
mail settings
Submitter Augie Fackler
Date May 29, 2017, 2:32 p.m.
Message ID <75e176c753d2c2c7eb5b.1496068343@imladris.local>
Download mbox | patch
Permalink /patch/21046/
State Accepted
Headers show

Comments

Augie Fackler - May 29, 2017, 2:32 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1496000969 14400
#      Sun May 28 15:49:29 2017 -0400
# Node ID 75e176c753d2c2c7eb5b5c0791bc993160fcb7b1
# Parent  aa333c1982abfe12a3940811d07468a286de93db
help: convert flag default to bytes portably

We were relying on %s using repr on (for example) integer values. Work
around that for Python 3 while preserving all the prior magic.
Martijn Pieters - May 29, 2017, 4:37 p.m.
So why not use `%r` instead of `%s` so `repr()` is used (or rather,
`ascii()` in Python 3)? That way you don't have to use this dance.

On 29 May 2017 at 15:32, Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1496000969 14400
> #      Sun May 28 15:49:29 2017 -0400
> # Node ID 75e176c753d2c2c7eb5b5c0791bc993160fcb7b1
> # Parent  aa333c1982abfe12a3940811d07468a286de93db
> help: convert flag default to bytes portably
>
> We were relying on %s using repr on (for example) integer values. Work
> around that for Python 3 while preserving all the prior magic.
>
> diff --git a/mercurial/help.py b/mercurial/help.py
> --- a/mercurial/help.py
> +++ b/mercurial/help.py
> @@ -84,7 +84,11 @@ def optrst(header, options, verbose):
>              so = '-' + shortopt
>          lo = '--' + longopt
>          if default:
> -            desc += _(" (default: %s)") % default
> +            # default is of unknown type, and in Python 2 we abused
> +            # the %s-shows-repr property to handle integers etc. To
> +            # match that behavior on Python 3, we do str(default) and
> +            # then convert it to bytes.
> +            desc += _(" (default: %s)") % pycompat.sysbytes(str(default))
>
>          if isinstance(default, list):
>              lo += " %s [+]" % optlabel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - May 29, 2017, 4:38 p.m.
> On May 29, 2017, at 12:37 PM, Martijn Pieters <mj@zopatista.com> wrote:
> 
> So why not use `%r` instead of `%s` so `repr()` is used (or rather, `ascii()` in Python 3)? That way you don't have to use this dance.

Because then if the default was a bytestring we’d get a b’’ in the output.

> 
> On 29 May 2017 at 15:32, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1496000969 14400
> #      Sun May 28 15:49:29 2017 -0400
> # Node ID 75e176c753d2c2c7eb5b5c0791bc993160fcb7b1
> # Parent  aa333c1982abfe12a3940811d07468a286de93db
> help: convert flag default to bytes portably
> 
> We were relying on %s using repr on (for example) integer values. Work
> around that for Python 3 while preserving all the prior magic.
> 
> diff --git a/mercurial/help.py b/mercurial/help.py
> --- a/mercurial/help.py
> +++ b/mercurial/help.py
> @@ -84,7 +84,11 @@ def optrst(header, options, verbose):
>              so = '-' + shortopt
>          lo = '--' + longopt
>          if default:
> -            desc += _(" (default: %s)") % default
> +            # default is of unknown type, and in Python 2 we abused
> +            # the %s-shows-repr property to handle integers etc. To
> +            # match that behavior on Python 3, we do str(default) and
> +            # then convert it to bytes.
> +            desc += _(" (default: %s)") % pycompat.sysbytes(str(default))
> 
>          if isinstance(default, list):
>              lo += " %s [+]" % optlabel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
> 
> 
> -- 
> Martijn Pieters
Martijn Pieters - May 29, 2017, 4:40 p.m.
Right, indeed. Would it be worth the effort to have `_(...)` return an
object with a custom `%` for cases like these? Not sure what this would
look like though.

On 29 May 2017 at 17:38, Augie Fackler <raf@durin42.com> wrote:

>
> > On May 29, 2017, at 12:37 PM, Martijn Pieters <mj@zopatista.com> wrote:
> >
> > So why not use `%r` instead of `%s` so `repr()` is used (or rather,
> `ascii()` in Python 3)? That way you don't have to use this dance.
>
> Because then if the default was a bytestring we’d get a b’’ in the output.
>
> >
> > On 29 May 2017 at 15:32, Augie Fackler <raf@durin42.com> wrote:
> > # HG changeset patch
> > # User Augie Fackler <raf@durin42.com>
> > # Date 1496000969 14400
> > #      Sun May 28 15:49:29 2017 -0400
> > # Node ID 75e176c753d2c2c7eb5b5c0791bc993160fcb7b1
> > # Parent  aa333c1982abfe12a3940811d07468a286de93db
> > help: convert flag default to bytes portably
> >
> > We were relying on %s using repr on (for example) integer values. Work
> > around that for Python 3 while preserving all the prior magic.
> >
> > diff --git a/mercurial/help.py b/mercurial/help.py
> > --- a/mercurial/help.py
> > +++ b/mercurial/help.py
> > @@ -84,7 +84,11 @@ def optrst(header, options, verbose):
> >              so = '-' + shortopt
> >          lo = '--' + longopt
> >          if default:
> > -            desc += _(" (default: %s)") % default
> > +            # default is of unknown type, and in Python 2 we abused
> > +            # the %s-shows-repr property to handle integers etc. To
> > +            # match that behavior on Python 3, we do str(default) and
> > +            # then convert it to bytes.
> > +            desc += _(" (default: %s)") % pycompat.sysbytes(str(default)
> )
> >
> >          if isinstance(default, list):
> >              lo += " %s [+]" % optlabel
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >
> >
> >
> > --
> > Martijn Pieters
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - May 29, 2017, 4:42 p.m.
> On May 29, 2017, at 12:40 PM, Martijn Pieters <mj@zopatista.com> wrote:
> 
> Right, indeed. Would it be worth the effort to have `_(...)` return an object with a custom `%` for cases like these? Not sure what this would look like though.

My bias is “no” because that feels a little too clever.

> 
> On 29 May 2017 at 17:38, Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote:
> 
> > On May 29, 2017, at 12:37 PM, Martijn Pieters <mj@zopatista.com <mailto:mj@zopatista.com>> wrote:
> >
> > So why not use `%r` instead of `%s` so `repr()` is used (or rather, `ascii()` in Python 3)? That way you don't have to use this dance.
> 
> Because then if the default was a bytestring we’d get a b’’ in the output.
> 
> >
> > On 29 May 2017 at 15:32, Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote:
> > # HG changeset patch
> > # User Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>>
> > # Date 1496000969 14400
> > #      Sun May 28 15:49:29 2017 -0400
> > # Node ID 75e176c753d2c2c7eb5b5c0791bc993160fcb7b1
> > # Parent  aa333c1982abfe12a3940811d07468a286de93db
> > help: convert flag default to bytes portably
> >
> > We were relying on %s using repr on (for example) integer values. Work
> > around that for Python 3 while preserving all the prior magic.
> >
> > diff --git a/mercurial/help.py b/mercurial/help.py
> > --- a/mercurial/help.py
> > +++ b/mercurial/help.py
> > @@ -84,7 +84,11 @@ def optrst(header, options, verbose):
> >              so = '-' + shortopt
> >          lo = '--' + longopt
> >          if default:
> > -            desc += _(" (default: %s)") % default
> > +            # default is of unknown type, and in Python 2 we abused
> > +            # the %s-shows-repr property to handle integers etc. To
> > +            # match that behavior on Python 3, we do str(default) and
> > +            # then convert it to bytes.
> > +            desc += _(" (default: %s)") % pycompat.sysbytes(str(default))
> >
> >          if isinstance(default, list):
> >              lo += " %s [+]" % optlabel
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org <mailto:Mercurial-devel@mercurial-scm.org>
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel <https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel>
> >
> >
> >
> > --
> > Martijn Pieters
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org <mailto:Mercurial-devel@mercurial-scm.org>
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel <https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel>
> 
> 
> 
> -- 
> Martijn Pieters
Yuya Nishihara - May 30, 2017, 12:41 p.m.
On Mon, 29 May 2017 10:32:23 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1496000969 14400
> #      Sun May 28 15:49:29 2017 -0400
> # Node ID 75e176c753d2c2c7eb5b5c0791bc993160fcb7b1
> # Parent  aa333c1982abfe12a3940811d07468a286de93db
> help: convert flag default to bytes portably
> 
> We were relying on %s using repr on (for example) integer values. Work
> around that for Python 3 while preserving all the prior magic.
> 
> diff --git a/mercurial/help.py b/mercurial/help.py
> --- a/mercurial/help.py
> +++ b/mercurial/help.py
> @@ -84,7 +84,11 @@ def optrst(header, options, verbose):
>              so = '-' + shortopt
>          lo = '--' + longopt
>          if default:
> -            desc += _(" (default: %s)") % default
> +            # default is of unknown type, and in Python 2 we abused
> +            # the %s-shows-repr property to handle integers etc. To
> +            # match that behavior on Python 3, we do str(default) and
> +            # then convert it to bytes.
> +            desc += _(" (default: %s)") % pycompat.sysbytes(str(default))

pycompat.bytestr() can be used instead (as long as default isn't a unicode
containing non-ASCII characters.)

Patch

diff --git a/mercurial/help.py b/mercurial/help.py
--- a/mercurial/help.py
+++ b/mercurial/help.py
@@ -84,7 +84,11 @@  def optrst(header, options, verbose):
             so = '-' + shortopt
         lo = '--' + longopt
         if default:
-            desc += _(" (default: %s)") % default
+            # default is of unknown type, and in Python 2 we abused
+            # the %s-shows-repr property to handle integers etc. To
+            # match that behavior on Python 3, we do str(default) and
+            # then convert it to bytes.
+            desc += _(" (default: %s)") % pycompat.sysbytes(str(default))
 
         if isinstance(default, list):
             lo += " %s [+]" % optlabel