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

Add support for custom paramLabel #441

Closed
bbottema opened this issue Aug 12, 2018 · 9 comments
Closed

Add support for custom paramLabel #441

bbottema opened this issue Aug 12, 2018 · 9 comments

Comments

@bbottema
Copy link
Contributor

bbottema commented Aug 12, 2018

Ok so I have almost everything sorted out, except for one limitation.

Here's what I'm using currently to define an OptionSpec:

OptionSpec.builder(cliDeclaredOptionSpec.getName())
                        .type(List.class)
                        .auxiliaryTypes(String.class)
                        .arity(String.valueOf(possibleOptionValues.size()))
                        .paramLabel(determineParamLabel(possibleOptionValues))
                        .build()

This is working well for me, except that for every arity the paramLabel is repeated.

The only way to currently solve this, is by defining the type as String with an arity of 0 or 1 depending on whether there are parameters at all or not:

OptionSpec.builder(cliDeclaredOptionSpec.getName())
                        .type(String.class)
                        .arity(possibleOptionValues.isEmpty() ? "0" : "1")
                        .paramLabel(determineParamLabel(possibleOptionValues))
                        .build()

This solves the paramLabel issue, but now I have an even bigger problem, because now I need to manually extract the param tokens from the single string parameter. This is rather complex, since whitespace can be a separator as well as a valid value (you would know!).

What would make the entire solution work for me if I could do the following:

OptionSpec.builder(cliDeclaredOptionSpec.getName())
                        .type(List.class)
                        .auxiliaryTypes(String.class)
                        .arity(String.valueOf(possibleOptionValues.size()))
                        .customParamLabel(determineParamLabel(possibleOptionValues)) // <----
                        .build()

customParamLabel then would be used as-is without repeating.

What do you think?

@remkop
Copy link
Owner

remkop commented Aug 12, 2018

The paramLabel is repeated for every arity by design: This explains to the end user how many parameters they need to supply.

You can supply a custom synopsis if the auto-generated synopsis becomes too detailed.

Can you show an example of why this doesn’t work well for your application?

@bbottema
Copy link
Contributor Author

You can supply a custom synopsis if the auto-generated synopsis becomes too detailed.

It is not only a problem in the synopsis, it is also under the Options: listing:

image

@remkop
Copy link
Owner

remkop commented Aug 13, 2018

Thanks for that example, now I understand.

It looks like what you really need is Composite Parameters (#358).

I will look into how easy it would be to provide a customParamLabel feature.

Meanwhile, let's take another look at what you can do now. Starting with --email:from, this option (at least conceptually) needs a single argument: the email address. This email address argument may have whitespace in it. The standard way to pass arguments with whitespace to an application is to quote the parameter. For a file, this is normal: --file="C:\Program Files\SomeApp\app.exe". Note the double quotes surrounding the file path with embedded spaces.

For the email this could be --email:from="Remko Popma <remkop@a.b.c>". This looks reasonable to me, what do you think?

The --email:replyingTo could be treated the same way, but perhaps better split up into --email:replyingTo and --email:replyingToAll? Anyway, I would take a single (quoted) parameter and split the parts in the application.

With future versions of picocli (when Composite Parameters (#358) are supported), users could leave off the quotes.

@bbottema
Copy link
Contributor Author

bbottema commented Aug 13, 2018

Thanks for thinking with me here Remko, I really appreciate it. Picocli is pretty great.

The --email:replyingTo could be treated the same way, but perhaps better split up into --email:replyingTo and --email:replyingToAll?

If I could reduce all parameters to single-value parameters, we wouldn't have this discussion in the first place :)

So that's a no go. I have so many possible options and variations (it's much more than just configuring from and replyTo addressess), splitting them up is going to explode the possibilities into a complete mess.

The only way this would work with OptionSpec type String is with the following usage:
--replyingTo="\".\the message.msg\" true \"third argument\""

See how quickly that becomes messy? And that's just for using it, extracting the three arguments is messy too, since the token separators are not straightforward; sometimes it's a set of quotes, sometimes it's a space, but only if it doesn't occur in a set of quotes.

As you say, I really need Composite Parameters. Or a customParamLabel.

@remkop
Copy link
Owner

remkop commented Aug 13, 2018

Understood. Thanks for the clarification.

bbottema added a commit to bbottema/picocli that referenced this issue Aug 15, 2018
bbottema added a commit to bbottema/picocli that referenced this issue Aug 15, 2018
remkop added a commit that referenced this issue Aug 17, 2018
bbottema added a commit to bbottema/picocli that referenced this issue Aug 17, 2018
bbottema added a commit to bbottema/picocli that referenced this issue Aug 17, 2018
…ntry and taking into account separator instead of using hardcoded space " "
bbottema added a commit to bbottema/picocli that referenced this issue Aug 18, 2018
bbottema added a commit to bbottema/picocli that referenced this issue Aug 19, 2018
bbottema added a commit to bbottema/picocli that referenced this issue Aug 21, 2018
remkop pushed a commit that referenced this issue Aug 25, 2018
remkop pushed a commit that referenced this issue Aug 25, 2018
…nd taking into account separator instead of using hardcoded space " "
remkop pushed a commit that referenced this issue Aug 25, 2018
remkop added a commit that referenced this issue Aug 25, 2018
…s` annotations; javadoc; remove commented out code

Still TODO: add unit tests for configuring usage help with hideParamSyntax via annotations
@remkop remkop added this to the 3.6 milestone Aug 25, 2018
@remkop
Copy link
Owner

remkop commented Aug 25, 2018

I merged the PR. Thanks for the contribution!

I will close this ticket after adding some unit tests for configuring usage help with hideParamSyntax via annotations.

I started to think about Composite Parameters some more. I want to make sure that the resulting solution covers your use case. Let's continue that discussion on ticket #358.

@bbottema
Copy link
Contributor Author

Was this in a release yet? Couldn't find it.

@remkop
Copy link
Owner

remkop commented Sep 24, 2018

Yes. In 3.6. Enjoy!

@remkop
Copy link
Owner

remkop commented Sep 24, 2018

Details are in the release notes:
https://github.com/remkop/picocli/releases#3.6.0-fixes

Thanks again for the contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants