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

Create new feature parameterAllowedBeforeEndOfOptions #2115

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
9 changes: 9 additions & 0 deletions docs/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,9 @@ assert(mixed.positional == Arrays.asList("param0", "param1", "param2", "param3")
assert(mixed.options == Arrays.asList("AAA", "BBB"))
----

Note that the mixing of positional parameters and options is configurable, see <<End of Options Behaviour>>.


=== Double dash (`--`)
When one of the command line arguments is just two dashes without any characters attached (`--`),
picocli interprets all following arguments as positional parameters, even arguments that match an option name.
Expand Down Expand Up @@ -5261,6 +5264,12 @@ java App -x -y -y=123
----


=== End of Options Behaviour
Since picocli 2.0, positional parameters can be specified anywhere on the command line and no longer need to follow the options.
Starting with v4.8.0, `CommandLine::setParameterAllowedBeforeEndOfOptions` can be set to false in order to restrict
the mixing of options and positional parameters. This option forces positional parameters to follow the <<Double dash (`--`),End of Options delimiter>>.


=== Toggle Boolean Flags
When a flag option is specified on the command line picocli will set its value to the opposite of its _default_ value.

Expand Down
50 changes: 47 additions & 3 deletions src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,33 @@ public CommandLine setUnmatchedOptionsArePositionalParams(boolean newValue) {
return this;
}

/** Returns whether positional parameters on the command line are allowed to occur before the special End of Options delimiter.
* The default is {@code true}.
* @return {@code true} positional parameters may occur anywhere on the command line, {@code false} if they must follow End of Options.
* @since 4.8.0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add @since 4.8.0 to the javadoc for all public methods that were added in this pull request?

(Picocli follows semantic versioning, so additional API means the next version number should be 4.8.0, not 4.7.6.)

*/
public boolean isParameterAllowedBeforeEndOfOptions() {
return getCommandSpec().parser().parameterAllowedBeforeEndOfOptions();
}

/** Sets whether positional parameters on the command line are allowed to occur before the special End of Options delimiter.
* The default is {@code true}.
* <p>The specified setting will be registered with this {@code CommandLine} and the full hierarchy of its
* subcommands and nested sub-subcommands <em>at the moment this method is called</em>. Subcommands added
* later will have the default setting. To ensure a setting is applied to all
* subcommands, call the setter last, after adding subcommands.</p>
* @param newValue the new setting. When {@code false}, positional parameters must follow the special End of Options delimiter.
* @return this {@code CommandLine} object, to allow method chaining
* @since 4.8.0
*/
public CommandLine setParameterAllowedBeforeEndOfOptions(boolean newValue) {
getCommandSpec().parser().parameterAllowedBeforeEndOfOptions(newValue);
for (CommandLine command : getCommandSpec().subcommands().values()) {
command.setParameterAllowedBeforeEndOfOptions(newValue);
}
return this;
}

/** Returns whether the end user may specify arguments on the command line that are not matched to any option or parameter fields.
* The default is {@code false} and a {@link UnmatchedArgumentException} is thrown if this happens.
* When {@code true}, the last unmatched arguments are available via the {@link #getUnmatchedArguments()} method.
Expand Down Expand Up @@ -8546,6 +8573,7 @@ public static class ParserSpec {
private boolean unmatchedArgumentsAllowed = false;
private boolean unmatchedOptionsAllowedAsOptionParameters = true;
private boolean unmatchedOptionsArePositionalParams = false;
private boolean parameterAllowedBeforeEndOfOptions = true;
private boolean useSimplifiedAtFiles = false;

/** Returns the String to use as the separator between options and option parameters. {@code "="} by default,
Expand Down Expand Up @@ -8596,6 +8624,7 @@ public boolean useSimplifiedAtFiles() {
public boolean splitQuotedStrings() { return splitQuotedStrings; }
/** @see CommandLine#isUnmatchedOptionsArePositionalParams() */
public boolean unmatchedOptionsArePositionalParams() { return unmatchedOptionsArePositionalParams; }
public boolean parameterAllowedBeforeEndOfOptions() { return parameterAllowedBeforeEndOfOptions; }
/**
* @see CommandLine#isUnmatchedOptionsAllowedAsOptionParameters()
* @since 4.4 */
Expand Down Expand Up @@ -8664,6 +8693,10 @@ public boolean useSimplifiedAtFiles() {
public ParserSpec unmatchedOptionsAllowedAsOptionParameters(boolean unmatchedOptionsAllowedAsOptionParameters) { this.unmatchedOptionsAllowedAsOptionParameters = unmatchedOptionsAllowedAsOptionParameters; return this; }
/** @see CommandLine#setUnmatchedOptionsArePositionalParams(boolean) */
public ParserSpec unmatchedOptionsArePositionalParams(boolean unmatchedOptionsArePositionalParams) { this.unmatchedOptionsArePositionalParams = unmatchedOptionsArePositionalParams; return this; }
/**
* @see CommandLine#setParameterAllowedBeforeEndOfOptions(boolean)
* @since 4.8.0*/
public ParserSpec parameterAllowedBeforeEndOfOptions(boolean allowParametersBeforeEndOfOptions) { this.parameterAllowedBeforeEndOfOptions = allowParametersBeforeEndOfOptions; return this; }
/**
* @see CommandLine#setAllowSubcommandsAsOptionParameters(boolean)
* @since 4.7.6-SNAPSHOT */
Expand Down Expand Up @@ -8699,14 +8732,16 @@ public String toString() {
"limitSplit=%s, overwrittenOptionsAllowed=%s, posixClusteredShortOptionsAllowed=%s, " +
"separator=%s, splitQuotedStrings=%s, stopAtPositional=%s, stopAtUnmatched=%s, " +
"toggleBooleanFlags=%s, trimQuotes=%s, " +
"unmatchedArgumentsAllowed=%s, unmatchedOptionsAllowedAsOptionParameters=%s, unmatchedOptionsArePositionalParams=%s, useSimplifiedAtFiles=%s",
"unmatchedArgumentsAllowed=%s, unmatchedOptionsAllowedAsOptionParameters=%s, " +
"unmatchedOptionsArePositionalParams=%s, allowParametersBeforeEndOfOptions=%s, useSimplifiedAtFiles=%s",
abbreviatedOptionsAllowed, abbreviatedSubcommandsAllowed, allowOptionsAsOptionParameters,
allowSubcommandsAsOptionParameters, aritySatisfiedByAttachedOptionParam, atFileCommentChar,
caseInsensitiveEnumValuesAllowed, collectErrors, endOfOptionsDelimiter, expandAtFiles,
limitSplit, overwrittenOptionsAllowed, posixClusteredShortOptionsAllowed,
separator, splitQuotedStrings, stopAtPositional, stopAtUnmatched,
toggleBooleanFlags, trimQuotes,
unmatchedArgumentsAllowed, unmatchedOptionsAllowedAsOptionParameters, unmatchedOptionsArePositionalParams, useSimplifiedAtFiles);
unmatchedArgumentsAllowed, unmatchedOptionsAllowedAsOptionParameters,
unmatchedOptionsArePositionalParams, parameterAllowedBeforeEndOfOptions, useSimplifiedAtFiles);
}

void initFrom(ParserSpec settings) {
Expand All @@ -8732,6 +8767,7 @@ void initFrom(ParserSpec settings) {
unmatchedArgumentsAllowed = settings.unmatchedArgumentsAllowed;
unmatchedOptionsAllowedAsOptionParameters = settings.unmatchedOptionsAllowedAsOptionParameters;
unmatchedOptionsArePositionalParams = settings.unmatchedOptionsArePositionalParams;
parameterAllowedBeforeEndOfOptions = settings.parameterAllowedBeforeEndOfOptions;
useSimplifiedAtFiles = settings.useSimplifiedAtFiles;
}
}
Expand Down Expand Up @@ -13659,7 +13695,11 @@ private void validateConstraints(Stack<String> argumentStack, List<ArgSpec> requ
for (UnmatchedArgsBinding unmatchedArgsBinding : getCommandSpec().unmatchedArgsBindings()) {
unmatchedArgsBinding.addAll(unmatched.clone());
}
if (!isUnmatchedArgumentsAllowed()) { maybeThrow(new UnmatchedArgumentException(CommandLine.this, Collections.unmodifiableList(parseResultBuilder.unmatched))); }
if (!isUnmatchedArgumentsAllowed()) {
String extraMsg = "";
if (!isParameterAllowedBeforeEndOfOptions()) { extraMsg = ". Positional parameters must follow the EndOfOptions delimiter '" + getEndOfOptionsDelimiter() + "'."; }
maybeThrow(new UnmatchedArgumentException(CommandLine.this, Collections.unmodifiableList(parseResultBuilder.unmatched), extraMsg));
}
Tracer tracer = CommandLine.tracer();
if (tracer.isInfo()) { tracer.info("Unmatched arguments: %s", parseResultBuilder.unmatched); }
}
Expand Down Expand Up @@ -13932,6 +13972,10 @@ private void processPositionalParameter(Collection<ArgSpec> required, Set<ArgSpe
if (tracer.isDebug()) {
tracer.debug("Parser is configured to treat all unmatched options as positional parameter", arg);}
}
if (!endOfOptions && !commandSpec.parser().parameterAllowedBeforeEndOfOptions()) {
handleUnmatchedArgument(args);
return;
}
int argIndex = parseResultBuilder.originalArgList.size() - args.size();
if (tracer.isDebug()) {
tracer.debug("[%d] Processing next arg as a positional parameter. Command-local position=%d. Remainder=%s", argIndex, position, reverse(copy(args)));}
Expand Down
171 changes: 171 additions & 0 deletions src/test/java/picocli/Issue2103.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
package picocli;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

/**
* Enhancement from issue 2103 enables or disables positional parameters before the EndOfOptions delimiter (such as "--").
*/
public class Issue2103 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add tests to verify that

  • the CommandLine.setParameterAllowedBeforeEndOfOptions method returns true by default
  • calling setParameterAllowedBeforeEndOfOptions also impacts the existing subcommands
  • when subcommands are added after setParameterAllowedBeforeEndOfOptions is called, these added subcommands have the default value.

Please also add tests for the public ParserSpec parameterAllowedBeforeEndOfOptions getter and setter methods.

There may also be existing tests that need to be fixed since the output of ParserSpec::toString looks different now (which is good, by the way).

see https://github.com/remkop/picocli/blob/main/src/test/java/picocli/SubcommandTests.java#L776

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two failing tests are probably caused by the change to ParserSpec::toString:

picocli.TracerTest > testTracingDebugWithSubCommands FAILED
    org.junit.ComparisonFailure at TracerTest.java:375

picocli.TracerTest > testDebugOutputForDoubleDashSeparatesPositionalParameters FAILED
    org.junit.ComparisonFailure at TracerTest.java:118


static class App implements Runnable {
@CommandLine.Option(names = "--optA") String optA;
@CommandLine.Parameters()
final List<String> list = new ArrayList<String>();
public void run() { }
}

static class SubCommand implements Runnable {
@CommandLine.Option(names = "--optB") String optB;
@CommandLine.Parameters()
final List<String> list = new ArrayList<String>();
public void run() { }
}

/**
* Original behavior allows positional parameters before and after EndOfOptions delimiter.
*/
@Test
public void testOriginalBehavior() {
App app = CommandLine.populateCommand(new App(), "--optA joe a b -- --optB c d".split(" "));
assertEquals("joe", app.optA);
assertEquals(Arrays.asList("a", "b", "--optB", "c", "d"), app.list);
}

/**
* The default value for allowing parameters prior to the End Of Options delimiter should be true
* in order to maintain backward compatibility with previous releases.
*/
@Test
public void testOriginalDefault() {
App app = new App();
CommandLine c = new CommandLine(app);
assertTrue(c.isParameterAllowedBeforeEndOfOptions());
//verify ParserSpec getter (should return same value as CommandLine setting
assertTrue(c.getCommandSpec().parser().parameterAllowedBeforeEndOfOptions());

// Toggle value for setting and verify (tests setter, and verifies getters)
c.getCommandSpec().parser().parameterAllowedBeforeEndOfOptions(false);
assertFalse(c.getCommandSpec().parser().parameterAllowedBeforeEndOfOptions());
assertFalse(c.isParameterAllowedBeforeEndOfOptions());
}

/**
* When ParameterAllowedBeforeEndOfOptions is disabled, the exit code for USAGE should be returned
* when positional parameters are found before EndOfOptions delimiter.
*/
@Test
public void testTriggerUsage() {
App app = new App();
int exitCode = new CommandLine(app)
.setParameterAllowedBeforeEndOfOptions(false)
.execute("--optA joe a b -- --optB c d".split(" "));
assertEquals(2, exitCode); // Should exit with USAGE since a and b are unmatched arguments
assertEquals("joe", app.optA);
assertEquals(Arrays.asList("--optB", "c", "d"), app.list);
}

/**
* Using a valid command line with ParameterAllowedBeforeEndOfOptions disabled, should correctly parse the options
* after the EndOfOptions delimiter as well as the valid options before the delimiter.
*/
@Test
public void testParameterAllowedBeforeEndOfOptions() {
App app = new App();
int exitCode = new CommandLine(app)
.setParameterAllowedBeforeEndOfOptions(false)
.execute("--optA joe -- --optB c d".split(" "));
assertEquals(0, exitCode);
assertEquals("joe", app.optA);
assertEquals(Arrays.asList("--optB", "c", "d"), app.list);
}

/**
* Subcommand tests for ParameterAllowedBeforeEndOfOptions.
*/
@Test
public void testParameterAllowedBeforeEndOfOptionsSubCommand1() {
class SubCommandZ implements Runnable {
@CommandLine.Option(names = "--optZ") String optZ;
@CommandLine.Parameters()
final List<String> list = new ArrayList<String>();
public void run() { }
}

CommandLine cl = new CommandLine(new App())
.addSubcommand("cmdA", new SubCommand())
.setParameterAllowedBeforeEndOfOptions(false)
.addSubcommand("cmdZ", new SubCommandZ());

// The ParameterAllowedBeforeEndOfOptions should apply to both main command and subcommands
// The extra "a1" after "jack" should be rejected.
assertEquals(2, cl.execute("--optA jill cmdA --optB jack a1 -- --optC c d".split(" ")));
// The extra "a2" after "jill" should be rejected
assertEquals(2, cl.execute("--optA jill a2 cmdA --optB jack -- --optC c d".split(" ")));
/* The extra "a3" after "hill" should NOT be rejected since the setParameterAllowedBeforeEndOfOptions was
called before the subcommand was added. */
assertEquals(0, cl.execute("--optA jill cmdZ --optZ hill a3 -- --optC c d".split(" ")));
}

/**
* Subcommand tests for ParameterAllowedBeforeEndOfOptions.
*/
@Test
public void testParameterAllowedBeforeEndOfOptionsSubCommand2() {
App app = new App();
SubCommand sub = new SubCommand();
int exitCode = new CommandLine(app)
.addSubcommand("cmdA", sub)
.execute("--optA jill cmdA --optB jack a -- --optC c d".split(" "));
// the extra "a" after "jack" should be accepted, because ParameterAllowedBeforeEndOfOptions was not set
assertEquals(0, exitCode);
assertEquals("jill", app.optA);
assertEquals("jack", sub.optB);
assertTrue(app.list.isEmpty());
assertEquals(Arrays.asList("a", "--optC", "c", "d"), sub.list);
}

/**
* Validate that the setParameterAllowedBeforeEndOfOptions triggers a new message for unmatched positional arguments.
*/
@Test
public void testUnmatchedArgumentMessageAsFalse() {
App app = new App();
CommandLine cl = new CommandLine(app)
.setParameterAllowedBeforeEndOfOptions(false);
try {
CommandLine.populateCommand(cl, "--optA joe a1 -- --optB c d".split(" "));
fail("Unmatched positional argument should have thrown exception");
} catch (CommandLine.UnmatchedArgumentException ex) {
assertEquals("Unmatched argument at index 2: 'a1'. Positional parameters must follow the EndOfOptions delimiter '--'.", ex.getMessage());
}
}

/**
* Verify that the setParameterAllowedBeforeEndOfOptions triggers the original message when not set.
*/
@Test
public void testUnmatchedArgumentMessageAsDefault() {
class UnmatchedApp implements Runnable {
@CommandLine.Option(names = "--optA") String optA;
public void run() { }
}

UnmatchedApp app = new UnmatchedApp();
CommandLine cl = new CommandLine(app);
try {
CommandLine.populateCommand(cl, "--optA joe a1".split(" "));
fail("Unmatched positional argument should have thrown exception");
} catch (CommandLine.UnmatchedArgumentException ex) {
assertEquals("Unmatched argument at index 2: 'a1'", ex.getMessage());
}
}
}
Loading
Loading