[rfc][icedtea-web] localizable Plugin+settings+fix+cleanup
Jiri Vanek
jvanek at redhat.com
Wed Sep 17 14:46:42 UTC 2014
...
>> + mkdir -p $(DOCS_DIR) ; \
>> + HTML_DOCS_TARGET_DIR=$(DOCS_DIR)/html ; \
>> + PLAIN_DOCS_TARGET_DIR=$(DOCS_DIR)/plain ; \
>> + MAN_DOCS_TARGET_DIR=$(DOCS_DIR)/man/ ; \
>
> Drop the trailing slash.
fixed
>
>> mkdir $$HTML_DOCS_TARGET_DIR ; \
>> mkdir $$PLAIN_DOCS_TARGET_DIR ; \
>> - mkdir -p $$MAN_DOCS_TARGET_DIR ; \
>> + mkdir $$MAN_DOCS_TARGET_DIR ; \
>
> Why did you remove the "-p" switch? It is probably best to have this switch almost always on in
> scripts to avoid running into errors just because of non-existing parent directories.
I prefer opposite. To not use -p. If some typo seek in, then it can create whole branch in some
really strange lcoation...
Before this dline created two levels of diretories. So there was -p.Now it does not.
however, the
-export DOCS_DIR=$(TOP_BUILD_DIR)/icedtea-web-docs
+export DOCS_DIR=$(TOP_BUILD_DIR)/icedtea-web-docs/$(FULL_VERSION)
...
- mkdir $(DOCS_DIR) ; \
+ mkdir -p $(DOCS_DIR) ; \
does.
so not "fixed"
>
>> [...]
>> diff -r e30d71ab91c6 netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java
>> --- a/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java Wed Sep 10 10:22:46 2014
>> -0400
>> +++ b/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java Fri Sep 12 15:21:39 2014
>> +0200
>> @@ -58,17 +58,17 @@
>> @SuppressWarnings("serial")
>> public class NetworkSettingsPanel extends JPanel implements ActionListener {
>>
>> - private DeploymentConfiguration config;
>> + private final DeploymentConfiguration config;
>>
>> private JPanel description;
>> - private ArrayList<JPanel> proxyPanels = new ArrayList<JPanel>(); // The stuff with editable
>> fields
>> + private final ArrayList<JPanel> proxyPanels = new ArrayList<>(); // The stuff with editable
>> fields
>>
>> /** List of properties used by this panel */
>> - public static String[] properties = { "deployment.proxy.type",
>> - "deployment.proxy.http.host",
>> - "deployment.proxy.http.port",
>> - "deployment.proxy.bypass.local",
>> - "deployment.proxy.auto.config.url", };
>> + public static String[] properties = { DeploymentConfiguration.KEY_PROXY_TYPE,
>> + DeploymentConfiguration.KEY_PROXY_HTTP_HOST,
>> + DeploymentConfiguration.KEY_PROXY_HTTP_PORT,
>> + DeploymentConfiguration.KEY_PROXY_BYPASS_LOCAL,
>> + DeploymentConfiguration.KEY_PROXY_AUTO_CONFIG_URL, };
>
> The formatting here should probably go here like this:
> public static String[] properties = {
> DeploymentConfiguration.KEY_PROXY_TYPE,
> DeploymentConfiguration.KEY_PROXY_HTTP_HOST,
> DeploymentConfiguration.KEY_PROXY_HTTP_PORT,
> DeploymentConfiguration.KEY_PROXY_BYPASS_LOCAL,
> DeploymentConfiguration.KEY_PROXY_AUTO_CONFIG_URL
> };
>
> Please also mind the comma after the last element.
Thank you!
Fixed and reformatted.
>
>> /**
>> * Creates a new instance of the network settings panel.
>> @@ -259,18 +259,23 @@
>> */
...
>> CBCheckSignedAppletDontMatchException = Signed applets are not allowed to run when their actual
>> Codebase does not match the Codebase specified in their manifest. Expected: {0}. Actual: {1}. See:
>> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html for details.
>> CBCheckSignedFail = Application Codebase does NOT match the Codebase specified in the
>> application's manifest, and this application is signed. You are strongly discouraged from running
>> this application. See:
>> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html for details.
>>
>> +#itweb-settings man (note, spaces (especially the one around @@ markup)are important due to man
>> pages markup
>> +ITWSintro= - view and modify settings for @BOLD_OPEN at javaws @BOLD_CLOSE at and the
>> @BOLD_OPEN at browser plugin at BOLD_CLOSE@
>
> What are those "@BOLD_OPEN@" and "@BOLD_CLOSE@" tags? Are they some sort of project specific markup
> language? If so, I am very unhappy with this. Where is the documentation for this? Does it handle @
> escapes correctly?
>
> Is "... @BOLD_OPEN at javaws @BOLD_CLOSE at and ..." actually correct?
Yes all those are correct. See the @@ explanation lower.
>
>> +ITWSsynops=command arguments
>> +IWSdescL1=is a command line and a GUI program to modify and edit settings used by the icedtea-web
>> implementation \
>> +of at BOLD_OPEN@ javaws @BOLD_CLOSE at and the @BOLD_OPEN at browser plugin at BOLD_CLOSE@.
>
> Why did you suddenly introduce broken lines or line continuing characters to Message.properties,
> since you have always been strictly against them?
You remember, do you? :)) Well its not that I'm 100% against, I like the i18ns and original file
keeping same style.
Also I don't like cutting on 80chars. Then maybe wee all can throw away our wide monitors... So
easily spoken - here i broke, because it seemed to me more readable.
so not "fixed"
>
> Is "... of at BOLD_OPEN@ javaws @BOLD_CLOSE at and ..." actually correct?
>
>> +IWSdescL2=If executed without any arguments, it starts up a GUI. Otherwise, it tries to do what
>> is specified in the argument.
>> +IWSdescL3=The command-line allows quickly searching, making a copy of and modifying specific
>> settings without having to hunt through a UI.
>> +IWSexampleL1=Show the GUI editor
>> +IWSexampleL2=Resets the value of `{0}` setting.
>
> Are the "`" characters literals in the man page or markups?
copypaste from my summary email:)
Originally there were the ' (" ' ") char, however, I noted, then If I put {n} into '' like '{0}' the
properties fiel do not evaluate it correctly. Same for " char and no escaping helped.. So I put
here ` char:(
Anyway - no markup mentioned. In original text it was just quoting - making it readable or similar.
So it have nothing to do with any markup, and AFAIK all plain, html and man output is ok with it.I
will be happy to change it if you dont like it ad know replacement.
>
>> +ITWSdefault=default
>> +IWSexampleL3=Known properties
>> +IWSexampleL31=(key, value and default value (if different)):
>> +IWSexampleL32=(key and default value):
>> +
>> +#itweb-plugin man (note, spaces (especially the one around @@ markup)are important due to man
>> pages markup
>> +ITWPintro=- allow to run @BOLD_OPEN at java applets @BOLD_CLOSE at in your favourite
>> @BOLD_OPEN at browser@BOLD_CLOSE@
>
> Check double spaces.
fixed. Thanx!
> Is "... run @BOLD_OPEN at java applets @BOLD_CLOSE at in ..." actually correct?
>
>> +ITWPsynopsL1=is working in your browser, once your browser knows about this files.
>> +ITWPsynopsL2=The {0} must be placed, or linked iniside specific direcotries. See {1}
>
> Spelling!
Fixed:(
>
>> +ITWPsynopsL3=@BOLD_OPEN@ Mozzila compliant browsers @BOLD_CLOSE at like Firefox, Midori, Epiphany,
>> Chrome or Chromium use:
>
> Spelling! The foundation's name is Mozilla!
fixed:(
>
>> +ITWPsynopsL4=@BOLD_OPEN@ Opera family browsers @BOLD_CLOSE at like Opera use:
>
> Please be careful when mentioning registered trademark and product names. If you mention them, you
> will probably need to include a statement on their legal status somewhere in the documentation for
> certain jurisdictions. Hence, it is best to avoid them. However, I am also aware that avoiding them
> completely is not always possible. Therefore, I strongly suggest to use the term "Mozilla
> compatible" only, wherever required. "NPAPI compatible" is also acceptable but far more technical
> and probably unknown to most users. "Mozilla compatible" covers all browsers currently supported by
> IcedTea-Web and avoids entanglements with too many entities. Because Mozilla is also the original
> author of the NPAPI, it describes the technical aspect sufficiently enough. And, although Mozilla is
> trademarked as well, the term would nevertheless reduce any legal risks to a minimum, especially
> given the Mozilla foundation's nature and history in free software. Of course, this does not relieve
> us of placing a legal notice in the documentation about Mozilla either, but reduces any efforts to
> just one entity.
Ouch that is the long one. What is conclusion? It really was not celar :( I added the"
+ITWPtrademarks=All browsers or company names in this section may be subjects of trademarks and/or
copyrights.
line to end of paragraph.
>
>> [...]
>> diff -r e30d71ab91c6 netx/net/sourceforge/jnlp/util/docprovider/ItwebPluginTextProvider.java
>> --- a/netx/net/sourceforge/jnlp/util/docprovider/ItwebPluginTextProvider.java Wed Sep 10
>> 10:22:46 2014 -0400
>> +++ b/netx/net/sourceforge/jnlp/util/docprovider/ItwebPluginTextProvider.java Fri Sep 12
>> 15:21:39 2014 +0200
>> @@ -38,6 +38,7 @@
>>
>> import java.io.IOException;
>> import net.sourceforge.jnlp.config.PathsAndFiles;
>> +import net.sourceforge.jnlp.runtime.Translator;
>> import net.sourceforge.jnlp.util.docprovider.formatters.formatters.Formatter;
>
> Generally speaking, I am really having trouble accepting this kind of custom formatter to
> IcedTea-Web. Actually, this goes for any project which is not specifically concerned with parsing a
> well specified language. Unfortunately, most developers greatly underestimate the prerequisites for
> a properly working formatter. The probability of not getting it right is far greater than one would
> initially assume. This starts with checking inputs and outputs, goes over properly handling encoded
> strings, and ends on handling escapes properly. The amount of testing required to get /and/ keep a
> formatter correct is just enormous. Then, it has to be documented and everybody who wants to add
> documentation on a new feature that he/she has just implemented is required to (a) know how the
> documentation generator works and (b) understand its formatting.
> So ultimately, we should get rid of this formatter and any @TAG@ tags. If you really need formatting
> tags for man pages then the documentation generator should be able to include them itself
> automatically. Otherwise, adding documentation becomes unnecessarily difficult and overly complex.
> It's a pity that you rushed to push the documentation generator...
>
Fixing all this paragraph is definitely for another changesets.
Copypasted from summary::
the @bold@ whatever@ stuff:
- It is placeholder for <b> and </b> in html or \n.B \n and /n in man. Tbh Right now I dont
know solid workaround
- I would like to get rid of of all this @@ markups. And I'm successfully doing so:)
- I Get rid of all except the @bold@, Which I found as necessary evil. But if workaround will be
found. I will be most happy to get rid of it.
..end of coopypaste:
Well yes, Actually exactly this was reason why I rushed with this. Yes this can be improved, but
also that can be improved! or that? But I do not wont to update so big patch until eternity.
So I now will be focusing on such a parts which are not considered nice on some ends.
The @bold@ substitution, I dont know the perfect cure. Maybe repalce by <b> and </b> which can be
repalced to plain or man as is now @bold@ @bold close@ ?
Yes I agree, "project specific markup" is not nice. And I'm(will) be working on removal of it.
No escaping is now done. And I'm not sure if it is wonted/needed in current state of things.
If it will become needed, it will be added.
Yes, testing is needed. I wont to add unittests, and also automatic tests by xmllint on resulting
html. I don't know how to validate man :( And not sure If I wont validate plain :)
One general answer to this.
I will be much more happy to hunt minor bugs in this generator, then check till madness three
similar - but based on different sources - types of documentation. Maybe the current formatters
are not perfect (without maybe,... thy are not even pretending to be complex. They are
single-purposed ones) but do they job. And I doubt some more text will be added in close or more
remote future. The context of static texts have changed in years only few times, and really only a
bit. It was the properties, files and switches which were changed significantly and *always*
forgotten to be documented and,. more horribly left also completely outdated. I really do not wont
to check those errors anymore. Rather this.
J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: localizabblePlugin+settings+fix+cleanup2.patch
Type: text/x-patch
Size: 16988 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140917/d4d93333/localizabblePluginsettingsfixcleanup2-0001.patch>
More information about the distro-pkg-dev
mailing list