[rfc][icedtea-web][itweb-settings] itweb-settings use Option Parser
Jiri Vanek
jvanek at redhat.com
Fri Sep 26 11:48:21 UTC 2014
Uff... Now you will hate me. I have changed my mind a bit, And i need we need to fix an ols parser a
bit.
At first inline
Rest innline
>
>
> itweb-settingsOptionParser-6.patch
>
>
> diff -r 6d62f68fb037 launcher/launchers.in
> --- a/launcher/launchers.in Mon Sep 22 17:10:07 2014 +0200
> +++ b/launcher/launchers.in Thu Sep 25 17:26:49 2014 -0400
> @@ -53,7 +53,7 @@
> *)
> ARGS[$j]="$1"
> j=$((j+1))
> - if [ "$1" = "-headless" ] ; then
> + if [[ "$1" =~ -*headless ]] ; then
> SPLASH="false"
> fi
> ;;
Thsi is good. Please push as separate changeset (please let aazores/jie to check the changelog
before push)
> diff -r 6d62f68fb037 netx/net/sourceforge/jnlp/OptionsDefinitions.java
> --- a/netx/net/sourceforge/jnlp/OptionsDefinitions.java Mon Sep 22 17:10:07 2014 +0200
> +++ b/netx/net/sourceforge/jnlp/OptionsDefinitions.java Thu Sep 25 17:26:49 2014 -0400
> @@ -73,14 +73,14 @@
> TRUSTNONE("-Xtrustnone","BOTrustnone"),
> JNLP("-jnlp","BOJnlp", NumberOfArguments.ONE),
> //itweb settings
> - NODASHHELP("help", "IBOHelp"),
> - LIST("list", "IBOList"),
> - GET("get", "name", "IBOGet"),
> - INFO("info", "name", "IBOInfo"),
> - SET("set", "name value", "IBOSet"),
> - RESETALL("reset", "all", "IBOResetAll"),
> - RESET("reset", "name", "IBOReset"),
> - CHECK("check", "name", "IBOCheck"),
> + NODASHHELP("-help", "IBOHelp"),
we dont need this ;) This isnow duplication of HELP (maybe only the properties message needs tuning
to suite both - if possible) otherwise wee need to go with some HELP2 (I prefer first)
> + 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),
> + RESETALL("-reset", "all", "IBOResetAll"),
> + RESET("-reset", "name", "IBOReset", NumberOfArguments.ONE_OR_MORE),
> + CHECK("-check", "name", "IBOCheck"),
Otherwise this is good.
> //policyeditor
> //-help
> FILE("-file", "policy_file", "PBOFile"),
> @@ -160,8 +160,9 @@
> OPTIONS.GET,
> OPTIONS.INFO,
> OPTIONS.SET,
> + OPTIONS.RESET,
> OPTIONS.RESETALL,
> - OPTIONS.RESET,
> + OPTIONS.HEADLESS,
> OPTIONS.CHECK
> });
> }
> diff -r 6d62f68fb037 netx/net/sourceforge/jnlp/controlpanel/CommandLine.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/CommandLine.java Mon Sep 22 17:10:07 2014 +0200
> +++ b/netx/net/sourceforge/jnlp/controlpanel/CommandLine.java Thu Sep 25 17:26:49 2014 -0400
> @@ -21,8 +21,6 @@
> import static net.sourceforge.jnlp.runtime.Translator.R;
>
> import java.io.IOException;
> -import java.util.ArrayList;
> -import java.util.Arrays;
> import java.util.List;
> import java.util.Map;
>
> @@ -37,16 +35,17 @@
> import net.sourceforge.jnlp.util.docprovider.TextsProvider;
> import net.sourceforge.jnlp.util.docprovider.formatters.formatters.PlainTextFormatter;
> import net.sourceforge.jnlp.util.logging.OutputController;
> +import net.sourceforge.jnlp.util.optionparser.OptionParser;
>
> /**
> * Encapsulates a command line interface to the deployment configuration.
> * <p>
> - * The central method is {@link #handle(String[])}, which calls one of the
> - * various 'handle' methods. The commands listed in {@link #allCommands} are
> + * The central method is {@link #handle()}, which calls one of the
> + * various 'handle' methods. The commands listed in OptionsDefinitions are
> * supported. For each supported command, a method handleCOMMANDCommand exists.
> * This method actually takes action based on the command. Generally, a
> * printCOMMANDHelp method also exists, and prints out the help message for
> - * that specific command. For example, see {@link #handleListCommand(List)}
> + * that specific command. For example, see {@link #handleListCommand()}
> * and {@link #printListHelp()}.
> * </p>
> * Sample usage:
> @@ -72,6 +71,7 @@
>
>
> DeploymentConfiguration config = null;
> + private static OptionParser optionParser;
>
> /**
> * Creates a new instance
> @@ -125,11 +125,11 @@
> /**
> * 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) {
> - if (args.contains("help")) {
> + public int handleListCommand() {
> + List<String> args = optionParser.getValues(OptionsDefinitions.OPTIONS.LIST);
> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.NODASHHELP)) {
> printListHelp();
> return SUCCESS;
> }
> @@ -169,33 +169,29 @@
> /**
> * Handles the 'get' command.
> *
> - * @param args the arguments to the get command
> * @return an integer representing success (SUCCESS) or error handling the
> * get command.
> */
> - public int handleGetCommand(List<String> args) {
> - if (args.contains("help")) {
> + public int handleGetCommand() {
> + List<String> args = optionParser.getValues(OptionsDefinitions.OPTIONS.GET);
> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.NODASHHELP)) {
> printGetHelp();
> return SUCCESS;
> }
>
> - if (args.size() != 1) {
> - printGetHelp();
> - return ERROR;
> - }
> -
> Map<String, Setting<String>> all = config.getRaw();
>
> - String key = args.get(0);
> - String value = null;
> - if (all.containsKey(key)) {
> - value = all.get(key).getValue();
> - OutputController.getLogger().printOutLn(value);
> - return SUCCESS;
> - } else {
> - OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, R("CLUnknownProperty", key));
> - return ERROR;
> + for (String key : args) {
> + String value = null;
> + if (all.containsKey(key)) {
> + value = all.get(key).getValue();
> + OutputController.getLogger().printOutLn(key+": "+value);
> + } else {
> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, R("CLUnknownProperty", key));
> + return ERROR;
> + }
> }
> + return SUCCESS;
> }
>
> /**
> @@ -210,39 +206,46 @@
> /**
> * Handles the 'set' command
> *
> - * @param args the arguments to the set command
> * @return an integer indicating success (SUCCESS) or error in handling
> * the command
> */
> - public int handleSetCommand(List<String> args) {
> - if (args.contains("help")) {
> + public int handleSetCommand() {
> + List<String> args = optionParser.getValues(OptionsDefinitions.OPTIONS.SET);
> + args = OptionParser.splitListOnEquals(args);
> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.NODASHHELP)) {
> printSetHelp();
> return SUCCESS;
> }
>
> - if (args.size() != 2) {
> + if (args.size() % 2 != 0) {
This is no go. This must be handeld by parser.
on optionParser.getValues(OptionsDefinitions.OPTIONS.SET);
parser must return you something "known"
your choice if it is some special object of you,which contains key+value, or values separated by =,
or always even numer of args (probably best). NBUt parser is who have to prepare it - and check
it. (==die when non even result is at the end or whatever)
> printSetHelp();
> return ERROR;
> }
> + String key = "";
> + String value;
>
> - String key = args.get(0);
> - String value = args.get(1);
> -
> - 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;
> + for (String arg : args) {
> + if (args.indexOf(arg) % 2 == 0) {
same
> + key = arg;
> + } else {
> + value = arg;
> + if (config.getRaw().containsKey(key)) {
> + Setting<String> old = config.getRaw().get(key);
... snip. simialr issues.
> + assertEquals("baseballbat", list.get(6));
> + }
> +
> }
> \ No newline at end of file
>
Well what crossed my mind.
at first - you must hanlde your set key=value in parser - that means new NUmberOfArguments value.
Some EVEN_NUMBER_OR_WITHEQUALCHAR. ok?
And then parser will figh tiwith it.
Then I was thinking about the multiple options versus limited options.
To introduce limited options, it really need new parameter into
OPTIONS(String option, String helperString, String decriptionKey, NumberOfArguments
numberOfArguments) {
not instead NumberOfArguments, you got me wrong. *new*.
So it will be OPTIONS(String option, String helperString, String decriptionKey, NumberOfArguments
numberOfArguments, ArgumentOccurence oc) {
where ArgumentOccurence is new enum with values like exactly one, one or more, unique.. or
similar. Eg Xclearcahce is good candidate for NON_OR_UNIQUE (as it have no sensein other cases...
And I'm still not sure with how to proceed with HELP - it clearly cannot be NONE_OR UNIQUE, but
should be :D )
But his would need to be tuned as separate changeset *before* tthis patch (as it is independent
patch toOptionPArser)
However, as javaws have 99% of arguments asked like hasOption() (except
-arg,-param -property", -update and -jnlp (and one null - the main arg)
and those args are handled in main in order, and javaws is terinaed accordingly, and... well this
sucks - only *first* of them have actually effect.
The second one is silently ignored (?) - except mainarg - in this case javaws is terminated.
On one side, it is really god to have so powerfull validation, on seconf, maybe it is to enforcing -
thoughts?
So I .. once javaws is so lenient to params, why not itweb-settings.
And, with your original idea of allmighty interpreter - it will be even *simple*
So my idea was.
You have
private final Map<OptionsDefinitions.OPTIONS, List<String>> parsedOptions;
Lets change it to list!
private final List<ParsedOption>
where parsed option is NewClass
contaiing
OptionsDefinitions.OPTIONS, name
and List<String> params
(dont forget about null mian param, and param-les soptions)
- you may also move some(all?) verification (on number of params) logic to this class
Now - you wil have to change hasOption method - but afaik it is all.
In this parsedOptions you naow have all items, including the order. And of course validated values
(both valid options only, and valid params only)
So the itweb-stting cmd clinet, should do nothhing more, then iterate through this array, and
evaluate each param....
Now - the best solution is probably *good* mixture of ArgumentOccurence and lanient list...I jsut
can not see it.
I'm on pto next week, please consult with jie/andrew/jacob/omair. Maybe in meantime you may adapt
PolicyEditor to parser - it is much simple. But I would liek to verify tthe itweb-setting option
parser before push. And of course you may discuse those list x ArgumentOccurence +/-
Sorry for neverending itterations, but I'm still somehow unhappy about the state of this future.
J.
More information about the distro-pkg-dev
mailing list