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 n-ary converter #1305

Closed
martlin2cz opened this issue Jan 11, 2021 · 6 comments
Closed

Add support for n-ary converter #1305

martlin2cz opened this issue Jan 11, 2021 · 6 comments

Comments

@martlin2cz
Copy link

Hello,

I would like to get following command created:

set-position 41 43

I have prepared the Command class with the parameters declaration as follows:

@Command(name = "set-position")
public class SetPositionCommand implements Runnable {
	@Parameters(arity = "2")
	private List<Integer> coords;

	// ... 
}

However, I would like to get the coords list automatically converted to my data class. Currently, I have to do it by hand:

	Point position = new Point(coords.get(0), coords.get(1));

I was thinking about utilizing the ITypeConverter, like so:

	@Parameters(arity = "2", converter = PointConverter.class)
	private Point position;

looking like:

public class PointConverter implements ITypeConverter<Point> {
	@Override
	public abstract T convert(List<String> values) throws Exception {
		return Point(Integer.parseInt(values.get(0)), Integer.parseInt(values.get(1)));
	}

But, the ITypeConverter maps 1 parameter to 1 object, not all of them into one.

Quite obviously, there would be new converter interface needed to be created (something like IMultiTypeConverter).

BTW is there any other workaround?

Thanks,
Martin

@remkop
Copy link
Owner

remkop commented Jan 11, 2021

Hi @martlin2cz, thanks for raising this.

Yes, this is possible. This can be accomplished with a parameter consumer.

Something like this:

@Parameters(parameterConsumer = PointConverter.class)
private Point position;

static class PointConverter implements IParameterConsumer {
    public void consumeParameters(Stack<String> args, ArgSpec argSpec, CommandSpec commandSpec) {
        if (args.size() < 2) {
            throw new ParameterException(commandSpec.commandLine(), 
                    "Missing coordinates for Point. Please specify 2 coordinates."));
        }
        int x = Integer.parseInt(args.pop());
        int y = Integer.parseInt(args.pop());
        argSpec.setValue(new Point(x, y));
    }
}

I will update the docs to add a link from the type conversion section.

@remkop
Copy link
Owner

remkop commented Jan 11, 2021

@martlin2cz I went ahead and added this section to the user manual: https://picocli.info/#_multi_parameter_type_converters

Can you verify and suggest improvements, if any?
Many thanks!

@remkop
Copy link
Owner

remkop commented Jan 12, 2021

Maybe the example would be better with a more explanatory paramLabel, what do you think?

@martlin2cz
Copy link
Author

Hello @remkop,

this is good news that there do exists some solution. Thanks for the doc update, I didn't know the parameter consumer can be used this way.

But, I'm thinking, wouldn't there still be possibility of some convient method in case like the mine? I mean in which the arity is fixed number. It would be nice to have the arity checked by the picocli automatically. Maybe just some abstract helper class like:


public abstract class ConsumerOfNaryParameter<T> implements IParameterConsumer {

	@Override
	public void consumeParameters(Stack<String> args, ArgSpec argSpec, CommandSpec commandSpec) {
		int argsCount = args.size();
		int minimalArgsCount = argSpec.arity().min();
		if (argsCount < minimalArgsCount) {
			 throw new ParameterException(commandSpec.commandLine(), 
	                    argSpec.paramLabel() + " requires at least " + minimalArgsCount + ", " 
	                    + "got " + argsCount);
		}
		
		int maximalArgsCount = argSpec.arity().max();
		List<String> values = args.subList(0, maximalArgsCount);
		T converted = convert(values);
		argSpec.setValue(converted);
	}
	
	public abstract T convert(List<String> values);
	
}

Would this be possible?

@remkop
Copy link
Owner

remkop commented Jan 15, 2021

It is always difficult to decide what belongs in the library and what belongs in applications.

The library currently offers the building blocks to build n-ary converters that do custom validation based on arity, or all kinds of other validations that we have not though of yet.

My initial take it to leave it like this for a while, at least until we see multiple requests and a clearer picture emerges of what additional features would help accomplish additional use cases. Does that make sense?

Taking a step back, how do you like picocli so far? Any particular likes and/or dislikes?
Don't forget to give picocli a star ⭐ on GitHub to help promote the project. 😉

@remkop remkop closed this as completed Jan 19, 2021
@martlin2cz
Copy link
Author

Okay, agree. Thanks.

@remkop remkop modified the milestones: 4.7, 4.6.2 Feb 23, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
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