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

Regularize the help system #10999

Closed
benjyw opened this issue Oct 20, 2020 · 18 comments
Closed

Regularize the help system #10999

benjyw opened this issue Oct 20, 2020 · 18 comments

Comments

@benjyw
Copy link
Sponsor Contributor

benjyw commented Oct 20, 2020

There are a few quirks with the current CLI help system:

  • Option help (and usage) is reached via ./pants help (for global options) and ./pants help <subsystem> for subsystem options.
  • Advanced option help is reached via ./pants help-advanced [<subsystem>].
  • Goal help is reached via ./pants goals.
  • Target help is reached via ./pants target-type, which is implemented as a goal, not as part of the help system.

I propose the following changes, for uniformity:

  • ./pants help shows the usage message, but no global option help. That way it fits on a single screen.
  • ./pants help goals shows goals.
  • ./pants help targets shows target types.
  • ./pants help <target_type> shows help for that target type.
  • ./pants help(-advanced) global shows global option help.
  • ./pants help(-advanced) <subsystem> shows help for the options in that subsystem.

Note that this means that goals, targets, global and target type aliases cannot be subsystem names. That seems like a good idea anyway to avoid confusion.

We had discussed having a convention where the subsystem target_type can be used for setting default values for field on the corresponding target_type. If we do so we can have ./pants help <target_type> express that special-case fact, but for now the prohibition on overlapping names above will actually help reserve the namespace for that possibility.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 20, 2020

Feedback please!

@Eric-Arellano
Copy link
Contributor

This is great, I love changing ./pants help to no longer output all the global options.

My main concern is

We had discussed having a convention where the subsystem target_type can be used for setting default values for field on the corresponding target_type.

We already now have a python-distribution subsystem. Are you proposing ./pants help python_distribution gives you the target, but ./pants help python-distribution the subsystem? That's subtle. Perhaps we should rename the subsystem and validate that the scopes don't clash, including when replacing - to _.

@gshuflin
Copy link
Contributor

Uniformity in this way strikes me as a good idea.

Right now pants also responds to the --help argument, as far as I can tell it prints exactly the same thing as ./pants help currently does. --help should either consistently do the same thing as help, or print out a message telling you exactly how pants help works, which should probably look pretty much like the blurb @benjyw posted in the above issue comment.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 21, 2020

We already now have a python-distribution subsystem. Are you proposing ./pants help python_distribution gives you the target, but ./pants help python-distribution the subsystem? That's subtle. Perhaps we should rename the subsystem and validate that the scopes don't clash, including when replacing - to _.

I don't see such a subsystem. Do you mean python-binary? That wouldn't clash with pex-binary, but it probably should...

Maybe it should simply be called pex-binary-defaults or something.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 21, 2020

Oh, it is already pex-binary.

@Eric-Arellano
Copy link
Contributor

options_scope = "python-distribution"

options_scope = "pex-binary"
deprecated_options_scope = "python-binary"

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 21, 2020

Huh, I had to pull to get that python-distribution subsystem in my client, so my git grep didn't find it.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 21, 2020

Either way, we can rename. And we should rename this one, since it's not (currently) setting a default value for a target-level field, which would be the rationale for having them share a name.

benjyw added a commit to benjyw/pants that referenced this issue Oct 21, 2020
Also reserves some names that cannot be used as scopes or aliases
('global', 'targets', 'goals') so that `./pants help <name>` is
unambiguous.

Works towards addressing pantsbuild#10999.

Also removes some cruft from test_extension_loader.py.

[ci skip-rust]

[ci skip-build-wheels]
benjyw added a commit to benjyw/pants that referenced this issue Oct 21, 2020
See pantsbuild#10999.

[ci skip-rust]

[ci skip-build-wheels]
benjyw added a commit that referenced this issue Oct 21, 2020
benjyw added a commit that referenced this issue Oct 21, 2020
Also reserves some names that cannot be used as scopes or aliases
('global', 'targets', 'goals') so that `./pants help <name>` is
unambiguous.

Works towards addressing #10999.

Also removes some cruft from test_extension_loader.py.
Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this issue Oct 21, 2020
Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this issue Oct 22, 2020
…uild#11004)

Also reserves some names that cannot be used as scopes or aliases
('global', 'targets', 'goals') so that `./pants help <name>` is
unambiguous.

Works towards addressing pantsbuild#10999.

Also removes some cruft from test_extension_loader.py.
@rcuza
Copy link
Contributor

rcuza commented Oct 22, 2020

This proposal highlights the potentially confusing location that pants plays for developers using a monorepo. One must learn how to use the tool pants and one must learn how to do development in a repository with pants. There is no way to avoid this, but luckily learning one helps with the other.

One of the great accomplishments of v2.0.0 is that it makes learning the tool pants more intuitive. This proposal is another step along that path.

It seems to me that the UI for pants can be summarized as pants <a_goal> [OPTIONS-FOR-GOAL] [targets]. That is there are no pants --options. Everything starts with choosing a goal.

If that is true, then the current behavior when one types pants is great:

 % ./pants
No goals specified.
Use `./pants goals` to list goals.
Use `./pants help` to get help.

And this proposal would probably simplify the output to two lines, getting rid of the "list goals" line. It might be worthwhile to be more verbose here to help those who are discovering pants. Maybe "Use ./pants help to get help on how to use and configure pants." would nudge people the right way.

Typing pants help is the first decision point. Does the user want help with setting up pants or help using pants to do work? The person typing the command may not even know. To help keep using pants as simple as possible, I would propose a couple of changes to the proposal:

  • Do not use the target "global". This has internal meaning to people developing pants (pants-builders) but I don't think that reaches a user's thinking. Create a target called "configure" or "options" that shows the current "Global options" and "Global advanced options".
  • Do not make help and help-advanced separate goals. There should not be multiple goals that do basically the same thing. Make --advanced an option for pants help.

Subsystems as a target for pants help is also too pants-builder centric, but I think it translate to the users perspective (but not perfectly). For instance with pants fmt and pants lint, "black" is a common subsystem. So if I type pants help black what do I expect to see? I am not sure. Maybe subsystem is not the right term. These things get enabled with backend_packages currently, the same place plugins get enabled. Are we using "backend", "subsystem" and "plugin" interchangeably? I think clarifying and simplifying this will make using pants more intuitive.

pants help goals and pants help targets give information on how to work with the monorepo. A newbie should be able to glean enough to get more help or be reminded of what they've read in READMEs or at https://pantsbuild.org/. I think this part of the proposal is great. The more detailed descriptions that shows when these commands are run should cover the different reasons a person typed it.

Something not mentioned is pants help-all. I believe it should be brought under this proposal. It would be great if every help had the option of outputting as json or whatever format pants supports. So it would become pants help all --output=[json|rst|txt|...]. The --output option would be available to all help targets.

The fewer the number of built in goals, the easier I think it will be for people and organizations to use pants. The more we build to a consistent pants UI or philosophy, the more intuitive the tool becomes.

@rcuza
Copy link
Contributor

rcuza commented Oct 22, 2020

I forgot about ./pants --level=debug. Are there other options that happen before the goal gets stated? (i.e. pants [OPTIONS] <a_goal>)

benjyw added a commit to benjyw/pants that referenced this issue Oct 22, 2020
Instead of it being the only help-related functionality
implemented as a standalone goal.

Also replaces `./pants goals` with `./pants help goals`,
and makes various other improvements to help, such as
not showing global option help when you just enter
`./pants help`, so that the usage message fits in a
single screen.

Addresses pantsbuild#10999.

[ci skip-rust]

[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor

Do not make help and help-advanced separate goals. There should not be multiple goals that do basically the same thing. Make --advanced an option for pants help.

I agree with this. One step further, I think completely get rid of help-advanced / help --advanced. help should always show the advanced options, but still as a separate "advanced" section.

It's far too subtle that you need to run ./pants help-advanced, and I fear users don't discover options because they don't realize they needed to use help-advanced instead of help.

Our help UI has greatly improved since a year ago; we now clearly delineate between normal options and advanced options. I don't think we'll confuse people by consolidating.

I forgot about ./pants --level=debug. Are there other options that happen before the goal gets stated? (i.e. pants [OPTIONS] <a_goal>)

Yes, e.g. ./pants --coverage-py-filter='project.my_module test ::. This is an option on the [coverage-py] subsystem, rather than on the test goal.

When you say ./pants dependencies --transitive ::, that is shorthand for ./pants --dependencies-transitive dependencies ::. Pants's CLI grammar is ./pants <fully-qualified options> <goal> <goal-options> <files/targets> <fully-qualified options>.

benjyw added a commit to benjyw/pants that referenced this issue Oct 22, 2020
Instead of it being the only help-related functionality
implemented as a standalone goal.

Also replaces `./pants goals` with `./pants help goals`,
and makes various other improvements to help, such as
not showing global option help when you just enter
`./pants help`, so that the usage message fits in a
single screen.

Addresses pantsbuild#10999.

[ci skip-rust]

[ci skip-build-wheels]
benjyw added a commit to benjyw/pants that referenced this issue Oct 22, 2020
Instead of it being the only help-related functionality
implemented as a standalone goal.

Also replaces `./pants goals` with `./pants help goals`,
and makes various other improvements to help, such as
not showing global option help when you just enter
`./pants help`, so that the usage message fits in a
single screen.

Addresses pantsbuild#10999.

[ci skip-rust]

[ci skip-build-wheels]
@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 22, 2020

Addressed in #11022

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 22, 2020

It seems to me that the UI for pants can be summarized as pants <a_goal> [OPTIONS-FOR-GOAL] [targets]. That is there are no pants --options. Everything starts with choosing a goal.

That's not quite true, because there are many global options. Under this proposal you would see them via ./pants help global.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 22, 2020

@rcuza Thanks for the feedback! ./pants help-all is indeed covered by my change, in that it now includes target help as well.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 22, 2020

Re getting rid of advanced help, I'm not so sure. It can get VERY verbose.

@rcuza
Copy link
Contributor

rcuza commented Oct 22, 2020

@benjyw pants help-all does a JSON dump of the help information. I was just pointing out that instead of making it a different goal, we could just make it an option under the help goal. I assume by its name it contains all the help information in one object.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 22, 2020

It does contain all the help info, exactly.

benjyw added a commit that referenced this issue Oct 22, 2020
Instead of it being the only help-related functionality
implemented as a standalone goal.

Also replaces `./pants goals` with `./pants help goals`,
and makes various other improvements to help, such as
not showing global option help when you just enter
`./pants help`, so that the usage message fits in a
single screen.

Addresses #10999.

[ci skip-rust]

[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor

This is now implemented through #11022 🚀

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

No branches or pull requests

4 participants