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

Improve help text for options accepting generic resources #387

Closed
thmo opened this issue Oct 6, 2021 · 10 comments · Fixed by #471
Closed

Improve help text for options accepting generic resources #387

thmo opened this issue Oct 6, 2021 · 10 comments · Fixed by #471
Labels
bug Something isn't working (template-set)
Milestone

Comments

@thmo
Copy link
Contributor

thmo commented Oct 6, 2021

Summary

The pulp rpm distribution create command has a --repository option. This options takes a generic resource as argument, i.e., [[<plugin>:]<resource_type>:]<name>' (albeit it would default to, and only accept rpm:rpm:). [NB: it does not accept a href either, not sure whether that could be added easily.]

Other commands however, e.g., pulp rpm repository content add only take the bare name of a repository as argument for --repository.

This seems to be a bit inconsistent and/or confusing.

If we do want to keep it the way it is, we should at least improve help texts.

Would a patch similar to this be acceptable?

diff --git a/pulpcore/cli/common/generic.py b/pulpcore/cli/common/generic.py
index 4271185..deef36a 100644
--- a/pulpcore/cli/common/generic.py
+++ b/pulpcore/cli/common/generic.py
@@ -361,6 +361,13 @@ def resource_option(*args: Any, **kwargs: Any) -> Callable[[_FC], _FC]:
         kwargs["callback"] = _multi_option_callback
     else:
         kwargs["callback"] = _option_callback
+
+    if "help" not in kwargs:
+        kwargs["help"] = _(
+            "Referenced resource, in the form [[<plugin>:]<resource_type>:]<name>. "
+            "'<plugin>' defaults to '{plugin}', '<resource> defaults to {resource_type}."
+        ).format(plugin=default_plugin or "", resource_type=default_type or "")
+
     return click.option(*args, **kwargs)
 
 

Still need to refine the help text in case the command in question accepts a href.

pulp-cli version info

pulp-cli 0.11.0-31-g591d6a6

@thmo thmo added the bug Something isn't working (template-set) label Oct 6, 2021
@mdellweg
Copy link
Member

mdellweg commented Oct 6, 2021

I was even considering to blindlessly append "either in the form <...> or by href" to the help text.

@mdellweg
Copy link
Member

mdellweg commented Oct 6, 2021

Be aware, that plugin and resource_type don't necessarily have a default.

@thmo
Copy link
Contributor Author

thmo commented Oct 6, 2021

Yes, hence the plugin=default_plugin or "", etc.

@daviddavis
Copy link
Contributor

daviddavis commented Oct 6, 2021

@thmo just wanted to chime in and say thank you for reporting these issues. They're very helpful in setting our priorities.

@mdellweg
Copy link
Member

mdellweg commented Oct 6, 2021

Yes, hence the plugin=default_plugin or "", etc.

Right. But i would skip the whole sentence if there is no default. Even more challenging: If there is no default we should not put the things in [] either.

@thmo
Copy link
Contributor Author

thmo commented Oct 6, 2021

What makes things a bit more complicated, is that the precise syntax is actually

[[[<plugin>]:][<type>]:]<name>

isn't it? Not sure if we really want to go that route...

@thmo
Copy link
Contributor Author

thmo commented Oct 6, 2021

@thmo just wanted to chime in and say thank you for reporting these issues. They're very helpful in setting our priorities.

Sure! Like the project and also the way it is organized (as far as I can tell from the outside, of course).

@mdellweg
Copy link
Member

mdellweg commented Oct 6, 2021

What makes things a bit more complicated, is that the precise syntax is actually

[[[<plugin>]:][<type>]:]<name>

isn't it? Not sure if we really want to go that route...

as long as they are optional, yes. If they aren't its really only <plugin>:<type>:<name>. But I agree. Let's try to keep each help text reasonably simple. The ::name can go into an example of the docs.

Are you up for casting this into a PR?

@mdellweg
Copy link
Member

mdellweg commented Feb 2, 2022

@thmo Wanted to ping you if you would be able to provide a PR.

@thmo
Copy link
Contributor Author

thmo commented Feb 4, 2022

Sorry totally forgot about this. Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working (template-set)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants