[rfc][icedtea-web] Option Parser

[Lukasz Dracz]

Hello,

> Looking quickly through the code, I am a little surprised. It seems like
> the simple/obvious thing to do is to create a new ParserSettings object
> based on settings to use for the parser (like, `ParserSettings(boolean
> strict)`), not from program arguments. I will be perfectly happy if you
> modified the ParserSettings class to move in this direction.
> 
> It's fine to do it in a separate patch.
> 
> Thanks,
> Omair

Alright, hmm ParserSettings already has a constructor with (boolean strict, boolean extensionAllowed, boolean malformedXmlAllowed) as parameters, the one difference between method and the constructor is that it assigns a new ParserSettings to the private static globalParserSettings, I'll look into it and see how this is all setup in a separate patch

>Please make both args and parsedOptions final as well

Okay

>Not a big fan of using the null key. Please add a comment to clarify that it is meant to be null and what it 
>represents.

Sure

>Moving the parsing of arguments into the constructor is fine but please extract it into it's own private function 
>or possibly their own functions and have them called in the constructor.

Sounds good

>This is a style nit but having this 'i--' in here requires more code-reading than say 'i = args.length - 1'. With 
>the latter it's more quickly understood that you are iterating args in reverse. Up to you;

Yeah I agree that makes it more readable.

>+        public String getLocalizedEscription() {
>typo?;

Yes, I have updated OptionsDescriptions to match the one released in Jiri's patch, some small differences from the one I posted before, which includes Jiri fixing this typo.

Thank you,
Lukasz Dracz

-------------- next part --------------
A non-text attachment was scrubbed...
Name: optionParser-11.patch
Type: text/x-patch
Size: 37258 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140904/c06c3c1e/optionParser-11-0001.patch>