[Changed Subscribers] D8639: py3: fix broken man page generation, it was generating `(default: NUL*)`

mjacob (Manuel Jacob) phabricator at mercurial-scm.org
Thu Jun 18 10:15:09 UTC 2020


mjacob added inline comments.

INLINE COMMENTS

> gendoc.py:90
> +            defaulttmpl = _(b" (default: %s)")
> +            if defaulttmpl:
> +                if not isinstance(default, bytes):

Isn’t `bool(defaulttmpl)` always true?

> gendoc.py:92
> +                if not isinstance(default, bytes):
> +                    default = repr(default).encode('latin1')
> +                desc += defaulttmpl % default

I think it’s safer to use 'ascii' here.

On Python 2, `repr(default)` is likely to return bytes and therefore will attempt to decode it with 'ascii' before encoding it with 'latin1'.

In general, I don’t think it is guaranteed that the output is expected to be latin1-encoded. If we ever have non-ASCII default values, we should fail early instead of producing mojibake.

An alternative to your code would be to use `pycompat.bytestr`. I don’t have a preference. An theoretical advantage is that is works correctly with `bytearray`, if we ever use it for default values. ;)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8639/new/

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

To: spectral, #hg-reviewers
Cc: mjacob, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200618/0ca62c42/attachment-0002.html>


More information about the Mercurial-patches mailing list