fallbackValue is not applied for vararg multi-value options (was: Support fallbackValue with arity="0..*")
#1904
Comments
|
I believe all the examples mentioned would work when the option is defined with I still need to think more about the general use case you mentioned… |
|
What comes to mind for the general case is that in the setter method, the logic could compare the parameter collection with the current value (the field where the collection is stored) and if both collections are equal, the end user didn’t specify a parameter. The logic can then programmatically add the fallback value. I haven’t tested this though, does it work? 😅 |
|
One more thing: I generally recommend against using The question to ask is “does my application really need to support cases like the below?” For the cases you mentioned above, arity 0..1 is sufficient. It still has some potential ambiguity. If this ambiguity becomes a problem, please check the section on Custom Parameter Processing. |
No, that is not a case I need to support, but maybe I misunderstood Interestingly, I can use an @Option(
names = { "--debug" },
paramLabel = "FACILITY", //
split = ",",
arity = "0..1", //
fallbackValue = "ALL", //
description = "The facilities for which to log debug information (one or more of ${COMPLETION-CANDIDATES}). "
+ "If none are given, debug information will be logged for ${FALLBACK-VALUE} categories.")
public void setDebugFacilities(Collection<DebugFacility> facilities) { ... }And then picocli still accepts the following as before (unit tests for the win!). Apparently, my mental model of But even when changing calls works and injects the I just don't get why picocli wouldn't, in the first case, either throw an
|
Not without further ugliness. ATM I treat Unfortunately, this means I cannot simply do this: public void setDebugFacilities(Set<DebugFacility> facilities) {
if (facilities.equals(this.debugCategories)) {
this.facilities = DebugFacility.values();
} else {
this.facilities = facilities;
}
}Even in the simplest case
Maybe a workaround is still possible by refining this further and being even more stateful in the |
Picocli's concept of arity is documented in the user manual: https://picocli.info/#_arity The section on Default Arity may also be of interest.
Just a note that what we are encountering here is the parser ambiguity I mentioned earlier. A parameter preprocessor may be needed to resolve this.
Not sure about the 2 calls or about why the setter is called with an empty collection when the value cannot be converted to a You can see a bit more what is going on internally by switching on tracing: https://picocli.info/#_tracing Can you post the output here? |
|
Sorry for the late replay; I was away for the holidays.
I created a test that reproduces this issue: sewe@7525c9c (feel free to copy it) Running the only test case which fails ( Hope this helps. At any rate, the test also nicely illustrates all cases that already do work, which is most of them 😃. |
|
Thanks for posting this. After looking at the debug trace and the code, I see now that this is an existing bug. AnalysisThe parser tries to determine for "vararg" options whether the next arg is an option parameter or not. The problem is that picocli has a quick check ( The fallbackValue is only applied if the quick check fails. <-- This is the bug... In this use case, the quick check passes, but the thorough check (the type conversion to enum value) fails. SolutionThe fix is also perform the thorough check when determining whether to apply the fallbackValue. TODO: add tests for collection and map options, for arities |
remkop
changed the title
fallbackValue is not applied for vararg multi-value options (was: Support fallbackValue with arity="0..*")
|
Tests: static class Issue1904 {
enum DebugFacility { FOO, BAR, BAZ, ALL, DEFAULT, FALLBACK }
@Option(
names = { "--debug" },
paramLabel = "FACILITY",
split = ",",
arity = "0..*",
defaultValue = "DEFAULT",
fallbackValue = "FALLBACK")
Collection<DebugFacility> facilities;
@Option(names = { "--map" }, arity = "0..*",
defaultValue = "DEFAULT=0",
fallbackValue = "FALLBACK=1")
Map<DebugFacility, String> map;
@Option(names = "-x")
String x;
@Parameters
List<String> remainder;
}
@Test
public void testIssue1904CollectionFallback_NoOptions() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904()); // options are not specified
assertEquals(Collections.singletonList(Issue1904.DebugFacility.DEFAULT), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.DEFAULT, "0"), obj.map);
}
@Test
public void testIssue1904CollectionFallback_NoArgs() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904(), "--debug"); // no args specified
assertEquals(Collections.singletonList(Issue1904.DebugFacility.FALLBACK), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.DEFAULT, "0"), obj.map);
}
@Test
public void testIssue1904CollectionFallback_NoArgs_map() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904(), "--map"); // no args specified
assertEquals(Collections.singletonList(Issue1904.DebugFacility.DEFAULT), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.FALLBACK, "1"), obj.map);
}
@Test
public void testIssue1904CollectionFallback_otherOption() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904(), "--debug", "-x", "xarg"); // other option
assertEquals(Collections.singletonList(Issue1904.DebugFacility.FALLBACK), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.DEFAULT, "0"), obj.map);
}
@Test
public void testIssue1904CollectionFallback_otherOption_map() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904(), "--map", "-x", "xarg"); // other option
assertEquals(Collections.singletonList(Issue1904.DebugFacility.DEFAULT), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.FALLBACK, "1"), obj.map);
}
@Test
public void testIssue1904CollectionFallback_positional() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904(), "--debug", "123"); // positional
assertEquals(Collections.singletonList(Issue1904.DebugFacility.FALLBACK), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.DEFAULT, "0"), obj.map);
}
@Test
public void testIssue1904CollectionFallback_positional_map() {
Issue1904 obj = CommandLine.populateCommand(new Issue1904(), "--map", "123"); // positional
assertEquals(Collections.singletonList(Issue1904.DebugFacility.DEFAULT), obj.facilities);
assertEquals(Collections.singletonMap(Issue1904.DebugFacility.FALLBACK, "1"), obj.map);
} |
|
The fix has been pushed to the main branch and will be included in the upcoming 4.7.1 release. |
Please support
fallbackValuewith "zero-based" arities other than0..1, in particular with0..*.To see why this might be useful, consider the following
@Optiondefinition:This should support the following scenarios
The last one unfortunately doesn't work, as the
fallbackValuedoes not get passed tosetDebugFacilities; it merely gets an emptyfacilitiesCollection.And we can't insert
ALLprogrammatically insetDebugFacilities, either. To see why, consider the following scenario:Here, the parameterless
--debugsimply results in another call ofsetDebugFacilitieswithFOOas single parameter. In other words, one cannot detect the parameterless caseMaybe there is a workaround that I am unable to see, but IMHO
fallbackValuesupport for any "zero-based" arity might be the most idiomatic solution. WDYT?The text was updated successfully, but these errors were encountered: