[rfc][icedtea-web] use Option Parser in itweb-settings
Jiri Vanek
jvanek at redhat.com
Mon Dec 8 16:13:06 UTC 2014
Ouch just found one mayor thing:
[jvanek at jvanek Desktop]$ ~/icedtea-web-image/bin/javaws aaa.aaa
Exception in thread "main" java.lang.NullPointerException
at
net.sourceforge.jnlp.util.optionparser.OptionParser.isEvenNumberSupportingEqualsChars(OptionParser.java:113)
at net.sourceforge.jnlp.util.optionparser.OptionParser.parseContents(OptionParser.java:82)
at net.sourceforge.jnlp.util.optionparser.OptionParser.<init>(OptionParser.java:68)
at net.sourceforge.jnlp.runtime.Boot.main(Boot.java:102)
will need fixing...
On 12/08/2014 02:31 PM, Jiri Vanek wrote:
> Yup! Realy cool work. I have only small nits, and the bigest ones I'm not sure how to deal:(
>
> minor nit (little bit unrelated, but should be included in patch):
>
> our docs says this:
> DESCRIPTION
> -check name - Checks that the current settings have valid values.(No argument expected)
> -get name - Shows the value of the named setting.(Expected one or more arguments)
> -headless - Disables download window, other UIs.(No argument expected)
> -help - Prints out information about supported command and basic usage.(No argument
> expected)
> -info name - Shows additional information about the named setting. Includes a description,
> the current value, the possible values, and the source of the setting.(Expected one or more arguments)
> -list - Shows a list of all settings.(No argument expected)
> -reset name - Resets the named setting to its original value.(Expected one or more arguments)
> -reset all - Resets all settings to their original values.(No argument expected)
> -set name value - Sets the setting to the new value, after checking that it is an appropriate
> value.(Expected even number of arguments with param=value as valid argument)
>
> You are affecting those, so try to align them with your real impl.
>
>
> main issues are - described functionality is really excellent, but the impl have few flaws:
>
> [jvanek at jvanek bin]$ ./itweb-settings check dsf sd
> No issues found.
>
> - sounds strnage doesnt it? But I'mnot sure if worthy to fix...
>
>
> [jvanek at jvanek bin]$ ./itweb-settings get dsf sd
> Unknown property-name "dsf"
> [jvanek at jvanek bin]$ ./itweb-settings get deployment.user.security.policy
> deployment.user.security.policy: file:///home/jvanek/.config/icedtea-web/security/java.policy
> [jvanek at jvanek bin]$ ./itweb-settings get deployment.user.security.policy dsf sfd
> deployment.user.security.policy: file:///home/jvanek/.config/icedtea-web/security/java.policy
> Unknown property-name "dsf"
>
> - so it dies with first unrecognized property... Not sure what to do here, wheter to keep it as
> you have it, or invest time and dye iumidiately (not worthy probably) , or after Unknown
> property-name just continue with other one... Whether the first is definitly the most correct, not
> sure if it is worthy....
>
>
> on contrary:
> [jvanek at jvanek bin]$ ./itweb-settings set dsf sfd
> WARNING: Unknown property name "dsf" - creating new property
> [jvanek at jvanek bin]$ ./itweb-settings set dsf sfd fdwf sfg
> Property "dsf" is unknown.
> WARNING: Unknown property name "fdwf" - creating new property
> [jvanek at jvanek bin]$ ./itweb-settings set dsf sfd fdw
> net.sourceforge.jnlp.util.optionparser.UnevenParameterException: For option -set expected an even
> number of params.
> at
> net.sourceforge.jnlp.util.optionparser.OptionParser.checkOptionHasEvenNumber(OptionParser.java:129)
> ....
>
> - this loks correct.But the previous issue may be more spread...
>
>
>
> --details
>
> well this is strange. On one side, it do not seem to work. On second, it seems to be undefined n
> general lsit of swithces, and so *undocumented*.
>
> Unless I missed some functionality, I'm for to drop it. (but if it have some, please tell before
> removing :))
>
>
>
>
>
>
> On 12/03/2014 09:24 PM, Lukasz Dracz wrote:
>> Hello,
>>
>> This patch changes itweb-settings to use the Option Parser. Only one itweb-settings command can be
>> used at a time in combinations with help and headless. Help specified with a command will return
>> the help for that command. The commands however have been changed to allow the use of multiple
>> arguments ex. itweb-settings get deployment.manifest.attributes.check
>> deployment.console.startup.mode deployment.log.file will get all three arguments. Added
>> functionality for EVEN_NUMBER_SUPPORTS_EQUALS_CHAR which is used in the set command so now it is
>> possible to enter arguments for the set as property=value or property value or a combination of this.
>>
>> Thank you,
>> Lukasz Dracz
>>
>>
>> itweb-settings-OptionParser-2.patch
>>
>>
>> diff --git a/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>> b/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>> --- a/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>> +++ b/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>> @@ -76,7 +76,7 @@
>> LIST("-list", "IBOList"),
>> GET("-get", "name", "IBOGet", NumberOfArguments.ONE_OR_MORE),
>> INFO("-info", "name", "IBOInfo", NumberOfArguments.ONE_OR_MORE),
>> - SET("-set", "name value", "IBOSet", NumberOfArguments.ONE_OR_MORE),
>> + SET("-set", "name value", "IBOSet", NumberOfArguments.EVEN_NUMBER_SUPPORTS_EQUALS_CHAR),
>
>
> pelase fix the messages properties to be accurate.
>
>> RESETALL("-reset", "all", "IBOResetAll"),
>> RESET("-reset", "name", "IBOReset", NumberOfArguments.ONE_OR_MORE),
>> /**
>> * Handles the 'list' command
>> *
>> - * @param args the arguments to the list command
>> * @return result of handling the command. SUCCESS if no errors occurred.
>> */
>> - public int handleListCommand(List<String> args) {
>
> The advantage of this declaration ^ is that it is more t
>
> You can always have:
> public int handleListCommand() {
> handleListCommand(optionParser)
> }
> public int handleListCommand(OptionParser op) {...
>
> But does nto meter, its up to you.
>
>> - if (args.contains("help")) {
>> + public int handleListCommand() {
>> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.HELP)) {
>> printListHelp();
>> return SUCCESS;
>> }
>>
>> boolean verbose = false;
>>
>> - if (args.contains("--details")) {
>> + if (optionParser.getMainArgs().contains("--details")) {
>
>
> hmhhm.. seesm nt to be working see top of email.
>
>
> (update 10 minutes later:)
>
> Well, aybe it may work, if istead of this --details, the itwebsettings recognize general verbose
> switch... what do you think?
>> verbose = true;
>> - args.remove("--details");
> ....
>> - old.getValidator().validate(value);
>> - } catch (IllegalArgumentException e) {
>> - OutputController.getLogger().log(OutputController.Level.WARNING_ALL,
>> R("CLIncorrectValue", old.getName(), value, old.getValidator().getPossibleValues()));
>> - OutputController.getLogger().log(e);
>> - return ERROR;
>> + for (String arg : args) {
>> + if (args.indexOf(arg) % 2 == 0) {
>
> ok, I dont understand this. What this 17 lines should do? Can ti be more readable? More "use
> parser" like? Or at least commented?
>
>> +
>> + key = arg;
>> + } else {
>> +
>> + value = arg;
>> + if (config.getRaw().containsKey(key)) {
>> + Setting<String> old = config.getRaw().get(key);
>> + if (old.getValidator() != null) {
>> + try {
>> + old.getValidator().validate(value);
>> + } catch (IllegalArgumentException e) {
>> + OutputController.getLogger().log(OutputController.Level.WARNING_ALL,
>> R("CLIncorrectValue", old.getName(), value, old.getValidator().getPossibleValues()));
>> + OutputController.getLogger().log(e);
>> + return ERROR;
>
> yes.. return or not return? Handle in parser? See lower...
>> + }
>> + }
>> +
>> + config.setProperty(key, value);
>> + } else {
>> + OutputController.getLogger().printOutLn(R("CLWarningUnknownProperty", key));
>> + config.setProperty(key, value);
>> }
>> }
>> - config.setProperty(key, value);
>> - } else {
>> - OutputController.getLogger().printOutLn(R("CLWarningUnknownProperty", key));
>> - config.setProperty(key, value);
>> }
>>
> .. big snip! :)
>> -
>> + public int handle() {
>> int val;
>> - if (command.equals(OptionsDefinitions.OPTIONS.HELP.option)) {
>> + if (getNumberOfOptions() > 1) {
>> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL,
>> R("CLUnexpectedNumberOfCommands"));
>> val = handleHelpCommand();
>
> I think those exceptions may be handeld in parser, but i dont insists (as help may have different
> meaning in javaws / itwebsettings/policy editor
>
> But if it does so, then our documentation will be always wrong... Maybe to have two different
> "help" declarations? I think it is possible to do so. One willbe used in javaws/polici editor and
> second (HELP1, help, info1, no argument)in itweb-settings (HELP2, help, info2, no or more args)
>
> Thougths?
>> - } else if (command.equals(OptionsDefinitions.OPTIONS.LIST.option)) {
>> - val = handleListCommand(arguments);
>> - } else if (command.equals(OptionsDefinitions.OPTIONS.SET.option)) {
>> - val = handleSetCommand(arguments);
>> - } else if (command.equals(OptionsDefinitions.OPTIONS.RESET.option)) {
>> - val = handleResetCommand(arguments);
>> - } else if (command.equals(OptionsDefinitions.OPTIONS.GET.option)) {
>> - val = handleGetCommand(arguments);
>> - } else if (command.equals(OptionsDefinitions.OPTIONS.INFO.option)) {
>> - val = handleInfoCommand(arguments);
>> - } else if (command.equals(OptionsDefinitions.OPTIONS.CHECK.option)) {
>> - val = handleCheckCommand(arguments);
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.LIST)) {
>> + val = handleListCommand();
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.SET)) {
>> + val = handleSetCommand();
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.RESET)) {
>> + val = handleResetCommand();
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.GET)) {
>> + val = handleGetCommand();
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.INFO)) {
>> + val = handleInfoCommand();
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.CHECK)) {
>> + val = handleCheckCommand();
>> + } else if (optionParser.hasOption(OptionsDefinitions.OPTIONS.HELP)) {
>> + val = handleHelpCommand();
>> } else {
>> - OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL,
>> R("CLUnknownCommand", command));
>> + for (String unknown : optionParser.getMainArgs()) {
>> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL,
>> R("CLUnknownCommand", unknown));
>> + }
>> handleHelpCommand();
>> val = ERROR;
>> }
>> -
>> return val;
>> }
>>
>> + private int getNumberOfOptions() {
>> + int number = optionParser.getNumberOfOptions();
>> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.HELP)) {
>> + number--;
>> + }
>> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.HEADLESS)) {
>> + number--;
>> + }
>> + return number;
>> + }
>> /**
>> * The starting point of the program
>> * @param args the command line arguments to this program
>> */
>> public static void main(String[] args) throws Exception {
>> - DeploymentConfiguration.move14AndOlderFilesTo15StructureCatched();
>> - if (args.length == 0) {
>> - ControlPanel.main(new String[] {});
>> - } else {
>> - CommandLine cli = new CommandLine();
>> - int result = cli.handle(args);
>> + try {
>> + optionParser = new OptionParser(args, OptionsDefinitions.getItwsettingsCommands());
>>
>> - // instead of returning, use JNLPRuntime.exit() so we can pass back
>> - // error codes indicating success or failure. Otherwise using
>> - // this program for scripting will become much more challenging
>> - JNLPRuntime.exit(result);
>> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.HEADLESS)) {
>> + JNLPRuntime.setHeadless(true);
>> + }
>> + DeploymentConfiguration.move14AndOlderFilesTo15StructureCatched();
>
> Not sure If I got this. It was missing here?? Crap.. It would be worthy for 1.5 too :(
>
>> + if (args.length == 0) {
>> + ControlPanel.main(new String[] {});
>> + } else {
>> + CommandLine cli = new CommandLine();
>> + int result = cli.handle();
>> +
>> + // instead of returning, use JNLPRuntime.exit() so we can pass back
>> + // error codes indicating success or failure. Otherwise using
>> + // this program for scripting will become much more challenging
>> + JNLPRuntime.exit(result);
>> + }
>> + } catch (Exception e) {
>> + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, e);
>> + JNLPRuntime.exit(1);
>> }
>> }
>> }
>> diff --git a/netx/net/sourceforge/jnlp/resources/Messages.properties
>> b/netx/net/sourceforge/jnlp/resources/Messages.properties
>> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties
>> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties
>> @@ -338,6 +338,9 @@
>> be added and selected. Multiple URLs may also be given with a single -codebase flag by
>> separating them with spaces. In this case, the last codebase given will be selected, and all will
>> be \
>> added. If this flag is given more than once, only the first is used.
>>
>> +# Option Parser
>> +OPUnevenParams=For option {0} expected an even number of params.
>> +
>> # NumberOfArguments descriptions.
>> NOAnone=No argument expected
>> NOAone=Exactly one argument expected
>> @@ -909,6 +912,7 @@
>> CLResetDescription=Resets the value for property-name to it\'s default value.\nall resets all
>> properties recognized by IcedTea-Web to their default value.
>> CLInfoDescription=Shows more information about the given property
>> CLCheckDescription=Shows any properties that have been defined but are not recognized by
>> IcedTea-Web
>> +CLUnexpectedNumberOfCommands=Itweb-settings can only run one command at a time.
>>
>> # splash screen related
>> SPLASHerror = Click here for details. An exception has occurred.
>> diff --git a/netx/net/sourceforge/jnlp/runtime/Boot.java
>> b/netx/net/sourceforge/jnlp/runtime/Boot.java
>> --- a/netx/net/sourceforge/jnlp/runtime/Boot.java
>> +++ b/netx/net/sourceforge/jnlp/runtime/Boot.java
>> @@ -46,6 +46,7 @@
>> import net.sourceforge.jnlp.util.logging.OutputController;
>> import net.sourceforge.jnlp.util.optionparser.InvalidArgumentException;
>> import net.sourceforge.jnlp.util.optionparser.OptionParser;
>> +import net.sourceforge.jnlp.util.optionparser.UnevenParameterException;
>> import sun.awt.AppContext;
>> import sun.awt.SunToolkit;
>>
>> @@ -96,7 +97,12 @@
>> * Launch the JNLP file specified by the command-line arguments.
>> */
>> public static void main(String[] argsIn) {
>> - optionParser = new OptionParser(argsIn, OptionsDefinitions.getJavaWsOptions());
>> + try {
>> + optionParser = new OptionParser(argsIn, OptionsDefinitions.getJavaWsOptions());
>> + } catch (UnevenParameterException upe) {
>> + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, upe);
>> + JNLPRuntime.exit(1);
>
> Wby exit? Isnt rethrow better? Why rethrow at all.. this exception may continue up and up until
> uttermost top end death, or not?
>
>> + }
>>
>> if (optionParser.hasOption(OptionsDefinitions.OPTIONS.VERBOSE)) {
>> JNLPRuntime.setDebug(true);
>> diff --git a/netx/net/sourceforge/jnlp/util/optionparser/OptionParser.java
>> b/netx/net/sourceforge/jnlp/util/optionparser/OptionParser.java
>> --- a/netx/net/sourceforge/jnlp/util/optionparser/OptionParser.java
>> +++ b/netx/net/sourceforge/jnlp/util/optionparser/OptionParser.java
>> @@ -41,6 +41,9 @@
>> import java.util.List;
>>
>> import net.sourceforge.jnlp.OptionsDefinitions;
>> +import net.sourceforge.jnlp.util.logging.OutputController;
>> +
>> +import static net.sourceforge.jnlp.runtime.Translator.R;
>>
>> /** Parses for options (passed in as arguments)
>> * To use add entries to OPTIONS enum in OptionsDefinitions
>> @@ -54,13 +57,18 @@
>> private final List<OptionsDefinitions.OPTIONS> possibleOptions;
>> //List of all possible main arguments
>> private final List<String> mainArgumentList = new ArrayList<>();
>> + private boolean evenNumberFound;
>>
>> - public OptionParser(String[] args, List<OptionsDefinitions.OPTIONS> options) {
>> + public OptionParser(String[] args, List<OptionsDefinitions.OPTIONS> options) throws
>> UnevenParameterException {
>> this.args = Arrays.copyOf(args, args.length);
>> this.possibleOptions = options;
>>
>> + evenNumberFound = false;
>> parsedOptions = new ArrayList<>();
>> parseContents();
>> + if (evenNumberFound) {
>> + checkOptionHasEvenNumber();
>> + }
>>
>> }
>>
>> @@ -71,12 +79,23 @@
>> lastOption = addOptionToList(arg);
>> } else if (shouldAddParam(lastOption)) {
>> lastOption.addParam(arg);
>> + } else if (isEvenNumberSupportingEqualsChars(lastOption)) {
>> + evenNumberFound = true;
>> + handleEvenNumberSupportingEqualsChar(lastOption, arg);
>> } else {
>> mainArgumentList.add(arg);
>> }
>> }
>> }
>>
>> + private void handleEvenNumberSupportingEqualsChar(final ParsedOption lastOption, String arg) {
>> + if (arg.contains("=")) {
>> + lastOption.addParam(arg.split("=")[0]);
>> + lastOption.addParam(arg.split("=")[1]);
>> + } else {
>> + lastOption.addParam(arg);
>> + }
>> + }
>> private boolean shouldAddParam(final ParsedOption lastOption) {
>> return lastOption != null &&
>> (oneOrMoreArguments(lastOption) || isOneArgumentNotFull(lastOption));
>> @@ -90,6 +109,10 @@
>> return lastOption.getOption().hasOneOrMoreArguments();
>> }
>>
>> + private boolean isEvenNumberSupportingEqualsChars(final ParsedOption lastOption) {
>> + return lastOption.getOption().hasEvenNumberSupportingEqualsChar();
>> + }
>> +
>> private ParsedOption addOptionToList(final String arg) {
>> ParsedOption option = new ParsedOption(argumentToOption(arg));
>> if (arg.contains("=")) {
>> @@ -99,6 +122,16 @@
>> return option;
>> }
>>
>> + private void checkOptionHasEvenNumber() throws UnevenParameterException {
>> + for (ParsedOption option : parsedOptions) {
>> + if (isEvenNumberSupportingEqualsChars(option)) {
>> + if (option.getParams().size() % 2 != 0){
>> + throw new UnevenParameterException(R("OPUnevenParams",
>> option.getOption().option));
>> + }
>> + }
>> + }
>> + }
>> +
>> private OptionsDefinitions.OPTIONS argumentToOption(final String arg) {
>> for (OptionsDefinitions.OPTIONS opt : possibleOptions) {
>> if (stringEqualsOption(arg, opt)) {
>> @@ -171,4 +204,8 @@
>> }
>> return result;
>> }
>> +
>> + public int getNumberOfOptions() {
>> + return parsedOptions.size();
>> + }
>
>
>
> Some tests of those new methods would be good.
>> }
>> diff --git a/netx/net/sourceforge/jnlp/util/optionparser/UnevenParameterException.java
>> b/netx/net/sourceforge/jnlp/util/optionparser/UnevenParameterException.java
>> new file mode 100644
>> --- /dev/null
>> +++ b/netx/net/sourceforge/jnlp/util/optionparser/UnevenParameterException.java
>> @@ -0,0 +1,43 @@
>> +/*Copyright (C) 2014 Red Hat, Inc.
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or
>> +modify it under the terms of the GNU General Public License as published by
>> +the Free Software Foundation, version 2.
>> +
>> +IcedTea is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to
>> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> +*/
>> +
>> +package net.sourceforge.jnlp.util.optionparser;
>> +
>> +public class UnevenParameterException extends Exception {
>> + public UnevenParameterException(String argument) {
>> + super(argument);
>> + }
>> +}
>> \ No newline at end of file
>>
>
>
> Nioce job! Looking forward to have this in!
>
>
> J.
More information about the distro-pkg-dev
mailing list