Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: cmdline: Use pydoc pager for --help #5344

Merged
merged 1 commit into from Jan 20, 2021

Conversation

kyleam
Copy link
Contributor

@kyleam kyleam commented Jan 19, 2021

The argument for -h/--help/--help-np is wired up with a custom
argparse.Action class, HelpAction. For an interactive call with
--help and a subcommand, this class looks for a corresponding manpage
file and tries to shell out to man(1), falling back to printing the
help string.

This means that for several common install scenarios, output isn't
displayed with a pager, and there is no distinction between --help and
--help-np. Given that the help text of most subcommands probably
exceeds common terminal heights, callers need to either scroll up or
do something like datalad <subcommand> --help | less.

Make a pager available in these situations by using pydoc.pager() to
display the --help output. Two notes:

  • guarding the use of pydoc.pager() with datalad's is_interactive()
    isn't essential because pydoc.pager() avoids using a pager when it
    detects a non-interactive environment. However, these interactive
    checks aren't identical, so it probably makes sense to honor a
    "no" from is_interactive().

  • pydoc.pager() will enter the pager even if the number of lines
    doesn't exceed the current terminal height. Given the length of
    the output for most subcommands, checking this ourselves (and
    worrying about platform-specific details) before calling
    pydoc.pager() probably isn't worth it.


I've had a draft for this around for a while and thought about it again as I was typing datalad siblings --help | less. Given the last point in the commit message, I'm not sure if others will think this is a good change. Thoughts?

The argument for -h/--help/--help-np is wired up with a custom
argparse.Action class, HelpAction.  For an interactive call with
--help and a subcommand, this class looks for a corresponding manpage
file and tries to shell out to man(1), falling back to printing the
help string.

This means that for several common install scenarios, output isn't
displayed with a pager, and there is no distinction between --help and
--help-np.  Given that the help text of most subcommands probably
exceeds common terminal heights, callers need to either scroll up or
do something like `datalad <subcommand> --help | less`.

Make a pager available in these situations by using pydoc.pager() to
display the --help output.  Two notes:

  * guarding the use of pydoc.pager() with datalad's is_interactive()
    isn't essential because pydoc.pager() avoids using a pager when it
    detects a non-interactive environment.  However, these interactive
    checks aren't identical, so it probably makes sense to honor a
    "no" from is_interactive().

  * pydoc.pager() will enter the pager even if the number of lines
    doesn't exceed the current terminal height.  Given the length of
    the output for most subcommands, checking this ourselves (and
    worrying about platform-specific details) before calling
    pydoc.pager() probably isn't worth it.
@codecov
Copy link

codecov bot commented Jan 19, 2021

Codecov Report

Merging #5344 (c7367f5) into master (58ae70a) will increase coverage by 30.25%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #5344       +/-   ##
===========================================
+ Coverage   59.64%   89.90%   +30.25%     
===========================================
  Files         129      300      +171     
  Lines       19045    42058    +23013     
===========================================
+ Hits        11360    37813    +26453     
+ Misses       7685     4245     -3440     
Impacted Files Coverage Δ
datalad/cmdline/helpers.py 66.91% <66.66%> (+28.93%) ⬆️
datalad/support/digests.py 100.00% <0.00%> (ø)
datalad/support/archive_utils_patool.py 29.87% <0.00%> (ø)
datalad/support/third/loris_token_generator.py 90.90% <0.00%> (ø)
datalad/tests/test_auto.py 88.75% <0.00%> (ø)
datalad/support/tests/test_digests.py 100.00% <0.00%> (ø)
datalad/customremotes/tests/test_archives.py 89.26% <0.00%> (ø)
datalad/customremotes/tests/test_ria_utils.py 96.87% <0.00%> (ø)
datalad/core/distributed/tests/test_push.py 99.31% <0.00%> (ø)
datalad/interface/ls_webui.py 94.54% <0.00%> (ø)
... and 259 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58ae70a...c7367f5. Read the comment docs.

Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks kosher. Didn't even know that there is more on windows.

On my attempt to use it I somehow do not even end up in that HelpAction, so yet to figure out that on my end

Copy link
Member

@mih mih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea!

@yarikoptic
Copy link
Member

especially exciting "will increase coverage by 30.25%" ... not sure what happened with coverage in master, but how could I resist this number? ;)

@yarikoptic yarikoptic merged commit 6945d71 into datalad:master Jan 20, 2021
@kyleam kyleam deleted the help-pager branch January 20, 2021 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants