Alon Bar-Lev has posted comments on this change. Change subject: uutils: added new cli parser ......................................................................
Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/40157/3/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java File backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java: Line 107: new IllegalArgumentException( Line 108: String.format("Value is required, but missing for argument '%1$s'", key) Line 109: ) Line 110: ); Line 111: } here Line 112: Object convertedValue = null; Line 113: if (value != null) { Line 114: Matcher m = parserArgument.getMatcher().matcher(value); Line 115: if (!m.find()) { Line 123: } Line 124: convertedValue = Util.getObjectValueByString(parserArgument.getConvert(), value); Line 125: } else if (parserArgument.getType() == ArgumentType.no_argument) { Line 126: convertedValue = Boolean.TRUE; Line 127: } why not just put the value as raw and have developer put the Boolean converter? this will enable us to do this much generic. no_argument converter is set to Boolean and value set to True. this means that we have no value so we assign "True" and then process the converter as usual and get True. this will enable us to also have optional_argument with default value, as --debug will for example put 5 as default while --debug=8 will leave 8. so before this matcher conditional there should be: if (value == null && parserArgument.getValue() != null) { value = parserArgument.getValue(); } and regular flow should follow. Line 128: Line 129: if (!parserArgument.isMultivalue()) { Line 130: argMap.put(key, convertedValue); Line 131: } else { Line 166: ); Line 167: } Line 168: if (arg.getType() == ArgumentType.no_argument) { Line 169: argMap.put(arg.getName(), arg.isValue()); Line 170: } not sure why we need this conditional. if I put no_argument as default False, value True and converter Boolean I can achieve the same in properties file without anything in special in implementation, no? Line 171: } Line 172: } Line 173: } Line 174: -- To view, visit https://gerrit.ovirt.org/40157 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00042b669e19293641579582223e7ca40717132d Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
