docs generator - prereview

[Jiri Vanek]

On 09/08/2014 07:39 PM, Jie Kang wrote:
> Hello,
>
> I haven't fully checked everything but it looks nice so far. The generated docs seem fine and the Makefile additions work for me. Just some very minor nits along with what aazores has said on the typos :D
>
> Patch review in-line with code below:
>
> diff -r c45cd47e962b netx/net/sourceforge/jnlp/OptionsDefinitions.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/OptionsDefinitions.java	Thu Sep 04 17:11:31 2014 +0200
> @@ -0,0 +1,181 @@
> ...
> +        //javaws undocummented swithces
> +        TRUSTALL("-Xtrustall","BOTrustall"),
> +        //javaws control-options
> +        ABOUT("-about", "BOAbout"),
> +        VIEWER("-viewer", "BOViewer"),
> +        CLEARCACHE("-Xclearcache", "BXclearcache"),
> +        LICENSE("-license", "BOLicense"),
> +        HELP("-help", "BOHelp"),
> +        //javaws run-options
> +        VERSION("-version", "BOVersion"),
> +        ARG("-arg", "arg", "BOArg", NumberOfArguments.ONE_OR_MORE),
> +        PARAM("-param", "name=value", "BOParam", NumberOfArguments.ONE_OR_MORE),
> +        PROPERTY("-property", "name=value", "BOProperty", NumberOfArguments.ONE_OR_MORE),
> +        UPDATE("-update", "seconds", "BOUpdate", NumberOfArguments.ONE),
> +        VERBOSE("-verbose", "BOVerbose"),
> +        NOSEC("-nosecurity", "BONosecurity"),
> +        NOUPDATE("-noupdate", "BONoupdate"),
> +        HEADLESS("-headless", "BOHeadless"),
> +        STRICT("-strict", "BOStrict"),
> +        XML("-xml", "BOXml"),
> +        REDIRECT("-allowredirect", "BOredirect"),
> +        NOFORK("-Xnofork", "BXnofork"),
> +        NOHEADERS("-Xignoreheaders", "BXignoreheaders"),
> +        OFFLINE("-Xoffline", "BXoffline"),
> +        TRUSTNONE("-Xtrustnone","BOTrustnone"),
> +        //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"),
> +        //policyeditor
> +        //-help
> +        FILE("-file", "policy_file", "PBOfile"),
> +        CODEBASE("-codebase", "url", "PBOcodebase");


Sure, I will focus on that. TY!

> For the values such as PBOcodebase, BOredirect, BOStrict, etc. can you make the capitalization consistent throughout?
> e.g. BOStrict vs BOstrict : up to you how to do it, but please make them all the same if possible.
> ...
> diff -r c45cd47e962b netx/net/sourceforge/jnlp/config/PathsAndFiles.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/netx/net/sourceforge/jnlp/config/PathsAndFiles.java	Thu Sep 04 17:11:31 2014 +0200
> @@ -0,0 +1,361 @@
> ...
> +    public static class InfrastructureFileDescriptor {
> I'm not a fan of multiple classes within one file, could you move all the various FileDescriptors into their own file?

Nope. If you look closely, all the subcalses are private, pnly making declarations of singletons 
more simple and readable.

The only exposed class is the public one, so I will keep this as it is.
> ...
> diff -r c45cd47e962b netx/net/sourceforge/jnlp/config/Defaults.java
> --- a/netx/net/sourceforge/jnlp/config/Defaults.java	Wed Aug 20 15:49:58 2014 -0400
> +++ b/netx/net/sourceforge/jnlp/config/Defaults.java	Thu Sep 04 17:11:31 2014 +0200
> @@ -37,47 +37,19 @@
> ...
>       /**
>        * Get the default settings for deployment
> @@ -85,10 +57,13 @@
>       public static Map<String, Setting<String>> getDefaults() {
>           SecurityManager sm = System.getSecurityManager();
>           if (sm != null) {
> -            sm.checkRead(userFile.toString());
> +            sm.checkRead(USER_DEPLOYMENT_FILE.getFullPath());
>           }
>
>
> +
> +
> +
> Are these three new-lines necessary? I think it looks a little awkward since there are now 5 new-lines in total.

:) good eye :)

> ...
> diff -r c45cd47e962b netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
> --- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Wed Aug 20 15:49:58 2014 -0400
> +++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Thu Sep 04 17:11:31 2014 +0200
> @@ -42,11 +42,7 @@
> +         aboutItwButtonAction = new ActionListener() {
> +            @Override
> +            public void actionPerformed(final ActionEvent e) {
> +                AboutDialog.display(false, TextsProvider.POLICY_EDITOR);
> This new addition to policy editor is bugged when launched from the security dialog. The AboutDialog for Icedtea-Web does not gain focus and cannot be manipulated (apart from moving the frame) until policyeditor is closed.
> Reproduce by:
> Run a java applet (http://docs.oracle.com/javase/tutorial/deployment/applet/deployingApplet.html) from browser such as firefox. When the security dialog appears asking for approval, open the menu and click "Launch policyeditor". From there, open Help -> About Icedtea-Web. Try to manipulate the dialog, for example, right-click on the frame and choose 'close'. Note that these actions do not work.
> Close policyeditor and note that the AboutDialog is now accessible.
>
> Using AboutDialog.display(true, ...); prevents this bug from occuring as the boolean represents modality of the Dialog.
>
>


Oh thats interesting. I will look into it, ty! And yes, the policy dialog is shown as modal in this 
case, so it have to be modal too. I will fix it by AboutDialog.display(policiedtior.ismodal(), ...);


J.