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

Issue #1535 - Add a help command (alias of --help) #1544

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

timacias
Copy link

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

  1. pipx --help and pipx [command_name] --help both produce help info.
  2. Issue help subcommand as an alias for --help #1535 requests a new pipx command that is an alias for --help.
  3. pipx help and pipx help [command_name] can now be used to provide identical output to (1).
  4. The help info now references the new help command.

Test plan

Tested by running

# command(s) to exercise these changes
python3 src/pipx help
python3 src/pipx help install
python3 src/pipx help pin

# if pipx is installed to your system
pipx help
pipx help install
pipx help pin

This is my first PR to a public repo. Please feel free to let me know if there are any guidelines or general tenets I've failed to follow. Thanks!

Initial addition of a `commands/help.py` file
Add "help" to `commands/__init__.py` to make pipx aware of a new command
Added an _add_help function which informs argparse of a "help" command, and an optional argument for it called "subcommand"
Added a check for args.command == "help"
Reflects requirements set by the linter
Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 💪 can you please add some tests for this?
Also can you verify this will work with commands with multiple subcommands, like pipx interpreter list?

src/pipx/main.py Outdated
@@ -973,7 +997,9 @@ def get_command_parser() -> Tuple[argparse.ArgumentParser, Dict[str, argparse.Ar
)
parser.man_short_description = PIPX_DESCRIPTION.splitlines()[1] # type: ignore[attr-defined]

subparsers = parser.add_subparsers(dest="command", description="Get help for commands with pipx COMMAND --help")
subparsers = parser.add_subparsers(
dest="command", description="Get help for commands with pipx COMMAND --help\n\t\t\t or pipx help COMMAND"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please stick to how the other commands wrap text in their description.

Example: `pipx help interpreter list` now has an identical output to `pipx interpreter list --help`
@timacias
Copy link
Author

Thanks for your comment. I have added a fix for cases with multiple subcomamnds (e.g., pipx interpreter list and pipx interpreter prune) . I'll add tests for the help subcommand in my next commit.

The line `valid_commands = parser._subparsers._actions[4].choices.keys()` causes the linter to throw an error:

```
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

tests/test_help.py:23: error: Item "None" of "_ArgumentGroup | None" has no attribute "_actions"  [union-attr]
tests/test_help.py:23: error: Item "Iterable[Any]" of "Iterable[Any] | Any | None" has no attribute "keys"  [union-attr]
tests/test_help.py:23: error: Item "None" of "Iterable[Any] | Any | None" has no attribute "keys"  [union-attr]
Found 3 errors in 1 file (checked 76 source files)
```

All linter tests pass normally when that line is substituted with a hard-coded list of valid pipx commands.
@timacias timacias requested a review from Gitznik September 27, 2024 03:14
@pypa pypa deleted a comment from totall4u Sep 28, 2024
Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

Generally, looks fine for me. Please make all pipelines pass, I'll give it another proper review once that's done.

Rather than accessing a protected attribute of `parser._subparsers`

Note: a commented out line with the protected access is still included in the event that the list of valid pipx commands changes
@timacias timacias requested a review from Gitznik October 5, 2024 06:21
Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

Just played around with this locally. If running pipx help, the autocomplete does not work anymore unfortunately. Could you have a look at that please?

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.

2 participants