[rfc][icedtea-web] localizable Plugin+settings+fix+cleanup
Jie Kang
jkang at redhat.com
Fri Sep 19 20:52:00 UTC 2014
----- Original Message -----
> ...
> >> + 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...
Hello,
I would also prefer a consistency in the makefile for mkdir using -p. When you use it in one place and not in another, it leads viewer to wonder why it's needed in one place and not in another. This means extra work to figure out the reason. Maybe some comments in-file here could help? If the only reason to not use '-p' is because typos would cause weird things to happen, then I would keep the -p. Typos shouldn't be a worry since we should in theory never accept things with typos.
I think some of the strings in Message.properties can be reworded to fix grammar mistakes and help formatting but that can be done in another patch. I will probably submit some changes to the strings myself :D
Apart from that it seems okay. I haven't thought of any good solutions to the issue with @BOLD_OPEN, etc. but I will keep thinking.
Regards,
>
> 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.
>
>
>
>
>
--
Jie Kang
More information about the distro-pkg-dev
mailing list