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

Suboptimal rendering of constraints in help output #182

Open
mathrick opened this issue Oct 5, 2024 · 3 comments
Open

Suboptimal rendering of constraints in help output #182

mathrick opened this issue Oct 5, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@mathrick
Copy link

mathrick commented Oct 5, 2024

Bug description

The way the constraints descriptions are printed under the option group headers doesn't work very well with longer descriptions that require wrapping. I have implemented a "required if missing" constraint as follows (RequireNamed is my custom constraint):

If(AnySet(*names), then=require_any.rephrased("optional"), else_=RequireNamed(*dependents))

and it results in the following output:

Asset metadata inputs:
  [optional if either --plugin or URL is set, otherwise parameters --title,
  --version, --godot_version, and --licence required]
  --title TEXT          Title / short description of the asset
  --version TEXT        Asset version
  --godot-version TEXT  Minimum Godot version asset is compatible with
  --licence TEXT        Asset's licence

Notice how the description wraps around in a way that makes it look like it's a part of the option listing. IMHO, there should be a blank line inserted between the description and the option list if the description wraps around.

To Reproduce

Steps to reproduce the behavior:

  1. Add the following constraint to any option group
constraint=If("some_option", then=require_any.rephrased("optional"), else_=require_all.rephrased(
              "this is some very long description that will wrap around and make things look weird"
             ))
  1. Run with --help
@mathrick mathrick added the bug Something isn't working label Oct 5, 2024
@janluke
Copy link
Owner

janluke commented Oct 7, 2024

Yeah, maybe an empty line could be added when the constraint is long.

Note that your constraint is probably wrong, as require_any doesn't mean "optional". In case your else_ branch actually require the full option group, the constraint could be rewritten as:

If(Not(AllSet(*names)), then=require_all)

which has a short description.

@janluke janluke added enhancement New feature or request and removed bug Something isn't working labels Oct 7, 2024
@mathrick
Copy link
Author

mathrick commented Oct 8, 2024

Right, I realised it actually reduced to require_all as I was writing this bug report, but it doesn't take away from the substance of it, since I actually have other constraints where the list of names is a subset of the available options, so it still suffers from the readability issues. This just happened to be one that had especially confusing output.

@janluke
Copy link
Owner

janluke commented Oct 8, 2024

Sure. But I'd recommend against using group constraints for a constraint that does NOT apply to the entire group. Use @constraint or the command help instead to instruct the user (or both). The correct use of an option group constraint is for constraint that apply to the full group and as such have a short description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants