pydoc: Use of MANPAGER variable is incorrect

[b7dd4af4-70e2-4b57-8e78-97c5f052a39d]

BPO	39400
Nosy	@serhiy-storchaka, @RPigott
PRs	python/cpython#18123

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-feature', 'library', '3.9'] title = 'pydoc: Use of MANPAGER variable is incorrect' updated_at = user = 'https://github.com/RPigott' ``` bugs.python.org fields: ```python activity = actor = 'Brocellous' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'Brocellous' dependencies = [] files = [] hgrepos = [] issue_num = 39400 keywords = ['patch'] message_count = 3.0 messages = ['360332', '360336', '361318'] nosy_count = 2.0 nosy_names = ['serhiy.storchaka', 'Brocellous'] pr_nums = ['18123'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue39400' versions = ['Python 3.9'] ```

[b7dd4af4-70e2-4b57-8e78-97c5f052a39d]

pydoc references the value of both MANPAGER and PAGER variables when selecting a command to present the user with documentation. Those values are passed directly to subprocess.Popen. However, MANPAGER may contain arguments that need splitting, and is explicitly documented as such in the man 1 man manpage:

If $MANPAGER or $PAGER is set ($MANPAGER is used in preference), its value is used as the name of the program used to display the manual page. [...] The value may be a simple command name or a command with arguments, and may use shell quoting (backslashes, single quotes, or double quotes). It may not use pipes to connect multiple commands; if you need that, use a wrapper script [...]

pydoc should perform word splitting a la shlex.split on the values of MANPAGER and PAGER to retain compatibility with man.

[b7dd4af4-70e2-4b57-8e78-97c5f052a39d]

Actually my previous comment is incorrect, the value of the PAGER is split with the shell as a result of shell=True. There _is_ still an incompatibility with man.

I have a value for MANPAGER set like 'env LESS_TERMCAP_mb=^[[01;31m ... less' which becomes split on ';' by the shell as a result of 'shell=True'. The MANPAGER value is not interpreted by a shell with man, so it works as expected when using man.

Becuase man explicitly allows the use of quotes, it is possible to set a value of MANPAGER that has the desired effect for both programs by quoting the terminal control sequences like so: 'env LESS="^[[01;31m ... less"'.

Still, it might be reasonable to change pydocs interpretation of MANPAGER to better align with man's.

[b7dd4af4-70e2-4b57-8e78-97c5f052a39d]

Any reason a change like this could not be added to a 3.8.x release?

Like I pointed out in the PR, pydoc's use of MANPAGER is undocumented and, as far as I'm concerned, incorrect.

A lot of software references idiosyncratic \<PROGRAM>_PAGER vars, but pydoc reuses MANPAGER instead of something like PYDOC_PAGER. I don't think it's such a bad idea to reuse MANPAGER considering the two programs serve a similar function, but I think matching man's interpretation of MANPAGER is then the only reasonable behavior pydoc can take. If it does not, pydoc is by-default broken on plenty of otherwise fine setups.

As it stands, pydoc is broken _because of undocumented behavior. A user reading the docs carefully still might not recognize that pydoc is preferring their MANPAGER value over PAGER. For that reason, I'd call this a bugfix. The bug here could be considered that pydoc references MANPAGER _at all. However, given what I think is the intent of the original code - to allow pydoc to benefit from a user's MANPAGER customization - I've tried to update pydoc to reflect that intent in the PR, rather than remove the feature.

I think this change could be eligible for 3.8.x. Thoughts?