Page MenuHomePhabricator

py3: fix broken man page generation, it was generating `(default: NUL*)`
ClosedPublic

Authored by spectral on Jun 17 2020, 8:15 PM.

Details

Summary

bytes(default) was producing things like (default: \x00) when handed
non-bytes values such as 1, 10, or True. The man page generation would
apparently ignore these bytes and produce man pages that had the string
(default: ).

Test Plan
  • Ran cd doc; python3 gendoc.py "hg.1.gendoc" and grepped for bad output
  • Ran make deb, extracted the deb, manually inspected hg.1 file.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

spectral created this revision.Jun 17 2020, 8:15 PM
mjacob added a subscriber: mjacob.Jun 18 2020, 6:15 AM
mjacob added inline comments.
doc/gendoc.py
91

Isn’t bool(defaulttmpl) always true?

93

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. ;)

spectral updated this revision to Diff 21663.Jun 18 2020, 1:50 PM
spectral added inline comments.Jun 18 2020, 1:51 PM
doc/gendoc.py
91

I assumed it wasn't and that this was a way of checking whether the translation had the string. But now that I'm reading it again, I think it's a different way of writing desc += _(b" (default: %s)") % bytes(default) if default else b"", so I've made this non-conditional.

93

Yeah, now that I look closer, I was confused by the use of latin1 above. Switched this to stringutil.forcebytestr.

mjacob accepted this revision.Jun 18 2020, 9:43 PM
mjacob added inline comments.
doc/gendoc.py
93

Sounds reasonable.

pulkit accepted this revision.Jun 19 2020, 4:32 AM
This revision is now accepted and ready to land.Jun 19 2020, 4:32 AM