Reviewer needed - fix for Re: Strange behaviour during javaws
Dr Andrew John Hughes
ahughes at redhat.com
Mon Mar 7 10:06:45 PST 2011
On 13:54 Mon 07 Mar , Jiri Vanek wrote:
> For Omair's notes:
>
> >This should not be required. Make will be running in
> >$(abs_top_builddir). Besides, each make rule executes in a new shell -
> >so the next line will not be affected by this cd command (look for
> >other examples of cd in the Makefile to see how that's done).
> >
> > + echo "<HTML><BODY><h1>News</h1><pre>">
> ${NETX_EXTRA_DIST_DIR}/news.html
> > + cat NEWS>> ${NETX_EXTRA_DIST_DIR}/news.html
> >
> >This fails in an out-of-tree build. Pleas use $(abs_top_srcdir)/NEWS
> >instead. This also needs to be done for README and AUTHORS.
>
> fixed.
>
> >This patch effectively removes notes.html and applications.html, >right?
>
> I have removed applications, kept notes (without authors), and add
> readme, news and authors.
>
Could we not leave applications.html in as documentation not used by the
about application directly, so we still provide users a list of demo
apps to try?
>
> >You should make explicitly make a note of methods (like
> >hyperlinkUpdate) which have been removed, as suggested in [1].
>
> Methods names added into changelog
>
> >Please dont add blank spaces within rules.
>
> fixed
>
>
>
> For Andrews notes
>
> >* There should really be a template, {news,readme,authors}.html.in
> >which then has
> >the relevant section replaced, rather than having all the html code
> >here. It
>
> I have added templates, but some problems raised, for which i have
> solution you will kick my head of...
>
> >may be worth looking at XSLT for a really nice way of doing it, but I
> >guess
>
> XSLT is not possible to be used with plaintext files.
> >sed would suffice, at least for a first bash.
>
True. Maybe we should consider using XML files as the source for AUTHORS
and NEWS and generating text and HTML versions. This would make it far
less likely that breaking changes are added to these files (particularly
NEWS) and would enable much more elaborate output without convoluted
programming or sed et. al. invocations.
> Oh I really wanted to do it by sed. But for entering file (which can
> contains characters which needed to be escaped) into another file
> becomes sed useless. Co I have added small class which do this
> replacement instead of too unreadable-n-times-escaped SEDing.
>
> >* If I read this correctly, it's on the end of the install target. It
> >shouldn't
> >be there. It should be a separate target generated during the build.
>
> I thing it is in right place. In this place all resources are copied, so
> why not even the generated ones?
This is the install stage which copies the stuff produced by make to its
final location. It shouldn't be creating new stuff. For one thing,
the about app will be broken until IcedTea-Web is installed.
>
>
> >* What is jamIcon.jpg?
> This icon have been presented in notes from its beginning. I have just
> used it instead in notes in authors. And it is handsome here;)
>
But where is it? Is it already in the repository?
>
> >I don't know what you're using to edit files, but you seem to be
> >producing some odd
> >whitespace and indentation changes in your patches.
>
> I'm, using netbeans. They do not use tab-character, and I consider it
> right - tab-character have nothing to do in java (notr for makefile!)
> code (except of \t in String). This files had tabs included from some
> old editor. I have not noticed it until patch dis-aligned.
>
The files look fine here. Please check what your editor is doing or
use a better one.
>
> I'm not sure if this Generator class which creates our generated html
> files is suitable solution, but it seams to me much nicer then sed
> command which escape all escapable sequences in plaintext (as \& is very
> ugly to escape).
>
It seems pretty convoluted. As I say above, I think switching the source
files to XML would allow us to produce nicer output with less fuss. Also,
as mentioned inline below, there are some issues with how you use this generator.
Can you attach examples of the generated output?
> Regards J.
> diff -r 0e868f1709f1 ChangeLog
> --- a/ChangeLog Thu Mar 03 09:39:41 2011 +0100
> +++ b/ChangeLog Mon Mar 07 13:26:04 2011 +0100
> @@ -1,3 +1,20 @@
> +2011-03-03 Jiri Vanek <jvanek at redhat.com>
> +
> + * Makefile.am: added generation of htmlfiles from NEWS and AUTHORS and
> + README
> + * extras/net/sourceforge/javaws/resources: authors.html, news.html,
> + readme.html added as templates for html pages generation during build
> + * extras/net/sourceforge/jnlp/about/Main.java: application tab replaced
> + with new tabs with generated files
> + * extras/net/sourceforge/jnlp/about/Generator.java: this class is used
> + for transfering README,NEWS and AUTHORS into theirs templates in
> + extras/net/sourceforge/javaws/resources as sed prove itself useless
> + for this task.
> + * extras/net/sourceforge/jnlp/about/Main.java: removed hyperlinkUpdate
> + and HyperlinkListener, as it can not work without all-permissions.
> + Also all createAndShowGUI was shorten for call from
> + net.sourceforge.jnlp package
> +
> 2011-03-02 Jiri Vanek <jvanek at redhat.com>
>
> * netx/net/sourceforge/jnlp/runtime/Boot.java: getAboutFile changed to
> diff -r 0e868f1709f1 Makefile.am
> --- a/Makefile.am Thu Mar 03 09:39:41 2011 +0100
> +++ b/Makefile.am Mon Mar 07 13:26:04 2011 +0100
> @@ -296,6 +296,10 @@
> ${INSTALL_DATA} -D $${files} \
> $(NETX_EXTRA_DIST_DIR)/$${files}; \
> done)
> +#generate news and authors to be used by javaws -about
> + java -cp $(abs_top_builddir)/extra-lib net.sourceforge.javaws.about.Generator -i=$(abs_top_builddir)/NEWS -t=${NETX_EXTRA_DIST_DIR}/news.html -o=${NETX_EXTRA_DIST_DIR}/news.html
> + java -cp $(abs_top_builddir)/extra-lib net.sourceforge.javaws.about.Generator -i=$(abs_top_builddir)/README -t=${NETX_EXTRA_DIST_DIR}/readme.html -o=${NETX_EXTRA_DIST_DIR}/readme.html
> + java -cp $(abs_top_builddir)/extra-lib net.sourceforge.javaws.about.Generator -i=$(abs_top_builddir)/AUTHORS -t=${NETX_EXTRA_DIST_DIR}/authors.html -o=${NETX_EXTRA_DIST_DIR}/authors.html -b="<BR/>"
This is still in the wrong place. You shouldn't use 'java' but $(JAVA). See the rest of the Makefile.am.
Also, where is Generator compiled? I don't think it should be installed but only used during the build.
> mkdir -p stamps
> touch $@
>
> diff -r 0e868f1709f1 extra/net/sourceforge/javaws/about/Generator.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/extra/net/sourceforge/javaws/about/Generator.java Mon Mar 07 13:26:04 2011 +0100
> @@ -0,0 +1,184 @@
> +/*
> + * To change this template, choose Tools | Templates
> + * and open the template in the editor.
> + */
This shouldn't be here.
> +
> +package net.sourceforge.javaws.about;
> +
> +import java.io.BufferedReader;
> +import java.io.BufferedWriter;
> +import java.io.File;
> +import java.io.FileInputStream;
> +import java.io.FileOutputStream;
> +import java.io.IOException;
> +import java.io.InputStreamReader;
> +import java.io.OutputStreamWriter;
> +
> +/**
> + * This class is more controlable replcaement for this bash script
> + *
> + * CONTENT=`cat INPUTPLAINTEXT` (or CONTENT=`cat INPUTPLAINTEXT | sed s/" "/ / | sed s/$/"<br>"/`)
> + * cat template.html | sed s/$WHAT_TO_REPLACE/$CONTENT/ > templateWithContent.html
> + *
> + *But because sed really don't like escape sequencing of escape sequencing, and plaintexts can really conatins anything, this class is usefull.
> + *
> + *
> + * @author jvanek
> + */
> +public class Generator {
> +
> + public static void main(String[] args) throws IOException{
> + if (args.length==0){
> + System.out.println("-b=? -s=? -m=? -i=inputfile -t=inputtemplate -o=outputfile");
> + System.out.println("-b - optional. end of lines in input ar ereplaced by value . Default is <br/>");
> + System.out.println("-s - optional. spaces in lines in input are replaced value. Default is ");
> + System.out.println("-o - optional. outputfile. Defoult is stdout.");
> + System.out.println("-i - optional. inputfile. Defouult is stdin.");
> + System.out.println("-r - optional. replacement. Default is [%content%].");
> + }
> + if (args.length<1){
> + System.out.println("at least one arg is expected - template");
> + return;
> + }
> +
> + Generator g=new Generator();
> + for (int i = 0; i < args.length; i++) {
> + String s = args[i];
> + s=s.replaceAll("^-*", "");
> + String[] ss= s.split(" *= *");
> + String key=ss[0].toLowerCase();
> + String value=null;
> + if (ss.length>1){
> + value=s.substring(key.length()+1);
> + value=value.replaceAll("^ *= *", "");
> + }
> + if (key.equals("b")){
> + if (value==null){
> + value="<br/>";
> + }
> + g.setEnding(value);
> + }
> + if (key.equals("s")){
> + if (value==null){
> + value=" ";
> + }
> + g.setSpacing(value);
> + }
> + if (key.equals("i")){
> + if (value==null){
> + System.out.println("input file without arg. ignored");
> + }else{
> + g.setInput(new File(value));
> + }
> + }
> + if (key.equals("r")){
> + if (value==null){
> + System.out.println("replacement without arg. ignored");
> + }else{
> + g.setReplacement((value));
> + }
> + }
> + if (key.equals("t")){
> + if (value==null){
> + throw new IllegalArgumentException("Fatal - template file cant not be set");
> + }
> + g.setTemplate(new File(value));
> + }
> + if (key.equals("o")){
> + if (value==null){
> + System.out.println("output file without arg. ignored");
> + }else {
> + g.setOutput(new File(value));
> + }
> + }
> +
> +
> +
> +
> + }
> +
> + g.proceed();
> +
> + }
> + private File input;
> + private File template;
> + private String ending;
> + private String spacing;
> + private String replacement="[%content%]";
> + private File output;
> +
> +
> + private void setEnding(String value) {
> + ending=value;
> + }
> +
> + private void setSpacing(String value) {
> + spacing=value;
> + }
> +
> + private void setInput(File file) {
> + input=file;
> + }
> +
> + private void setTemplate(File file) {
> + template=file;
> + }
> +
> + private void setOutput(File file) {
> + output=file;
> + }
Is there a point to all these set methods?
> +
> + private void proceed() throws IOException {
> + String inputS=readFile(input,true);
> + String templateS=readFile(template,false);
> + save(output,templateS.replace(replacement, inputS));
> +
> + }
> +
> + private String readFile(File input,boolean replace) throws IOException{
> + StringBuilder sb=new StringBuilder();
> + BufferedReader br=null;
> + if (input!=null){
> + br=new BufferedReader(new InputStreamReader(new FileInputStream(input), "utf-8"));
> + }else{
> + br=new BufferedReader(new InputStreamReader(System.in, "utf-8"));
> + }
> +while(true){
> + String s=br.readLine();
> + if(s==null) break;
> + if (replace){
> + if(spacing!=null) s=s.replaceAll(" ", spacing);
> + sb.append(s);
> + if (ending==null)sb.append("\n"); else sb.append(ending);
> + }else{
> + sb.append(s).append("\n");
> + }
> +
> +}
> + return sb.toString();
> + }
> +
> + private void save(File output,String s)throws IOException {
> + BufferedWriter br=null;
> + if (output!=null){
> + br=new BufferedWriter(new OutputStreamWriter(new FileOutputStream(output), "utf-8"));
> + }else{
> + br=new BufferedWriter(new OutputStreamWriter(System.out, "utf-8"));
> + }
> + try{
> + br.write(s);
> + }finally{
> + br.flush();
> + br.close();
> + }
> + }
> +
> +
> +
> +
> + private void setReplacement(String value) {
> + replacement=value;
> + }
> +
> +
> +}
> diff -r 0e868f1709f1 extra/net/sourceforge/javaws/about/Main.java
> --- a/extra/net/sourceforge/javaws/about/Main.java Thu Mar 03 09:39:41 2011 +0100
> +++ b/extra/net/sourceforge/javaws/about/Main.java Mon Mar 07 13:26:04 2011 +0100
> @@ -47,40 +47,43 @@
> import javax.swing.JPanel;
> import javax.swing.JTabbedPane;
> import javax.swing.UIManager;
> -import javax.swing.event.HyperlinkEvent;
> -import javax.swing.event.HyperlinkListener;
>
> -import net.sourceforge.jnlp.Launcher;
> -import net.sourceforge.jnlp.runtime.JNLPRuntime;
>
> -public class Main extends JPanel implements HyperlinkListener {
> +public class Main extends JPanel {
>
> - private final String notes = "/net/sourceforge/jnlp/about/resources/notes.html";
> - private final String apps = "/net/sourceforge/jnlp/about/resources/applications.html";
> - private final String about = "/net/sourceforge/jnlp/about/resources/about.html";
> + private final String notes = "/net/sourceforge/javaws/about/resources/notes.html";
> + private final String about = "/net/sourceforge/javaws/about/resources/about.html";
> + private final String news = "/net/sourceforge/javaws/about/resources/news.html";
> + private final String readme = "/net/sourceforge/javaws/about/resources/readme.html";
> + private final String authors = "/net/sourceforge/javaws/about/resources/authors.html";
> +
> JTabbedPane tabbedPane;
>
> public Main() throws IOException {
> super(new BorderLayout());
>
> - HTMLPanel notesPanel = new HTMLPanel(getClass().getResource(notes));
> - HTMLPanel appsPanel = new HTMLPanel(getClass().getResource(apps));
> - HTMLPanel aboutPanel = new HTMLPanel(getClass().getResource(about));
> + HTMLPanel notesPanel = new HTMLPanel(getClass().getResource(notes));
> + HTMLPanel aboutPanel = new HTMLPanel(getClass().getResource(about));
> + HTMLPanel newsPanel = new HTMLPanel(getClass().getResource(news));
> + HTMLPanel readmePanel = new HTMLPanel(getClass().getResource(readme));
> + HTMLPanel authorsPanel = new HTMLPanel(getClass().getResource(authors));
>
> - appsPanel.pane.addHyperlinkListener(this);
> +
>
> tabbedPane = new JTabbedPane();
>
> - tabbedPane.add("About NetX", aboutPanel);
> - tabbedPane.add("Applications", appsPanel);
> - tabbedPane.add("Notes", notesPanel);
> + tabbedPane.add("About NetX", aboutPanel);
> + tabbedPane.add("Notes", notesPanel);
> + tabbedPane.add("News", newsPanel);
> + tabbedPane.add("Readme", readmePanel);
> + tabbedPane.add("Authors", authorsPanel);
>
> tabbedPane.setPreferredSize(new Dimension(550,410));
> add(tabbedPane, BorderLayout.CENTER);
> }
>
> private static void createAndShowGUI() {
> - JNLPRuntime.setExitClass(Main.class);
> +
This seems to be an unrelated change which should be in a separate patch.
>
> try {
> UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
> @@ -119,17 +122,5 @@
> });
> }
>
> - public void hyperlinkUpdate(HyperlinkEvent e) {
> - if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED) {
> - URL url = e.getURL();
> -
> - Launcher launcher = new Launcher(
> - JNLPRuntime.getDefaultLaunchHandler());
> - try {
> - launcher.launchBackground(url);
> - } catch (Exception ex) {
> - ex.printStackTrace();
> - }
> - }
> - }
> +
> }
> diff -r 0e868f1709f1 extra/net/sourceforge/javaws/about/resources/authors.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/extra/net/sourceforge/javaws/about/resources/authors.html Mon Mar 07 13:26:04 2011 +0100
> @@ -0,0 +1,5 @@
> +<HTML><BODY><h1>Authors</h1>
> +<div align="center"><img src="jamIcon.jpg" alt="Jam Icon" width="87" height="84" align="Center"/><br/>
> +[%content%]
> +</div>
> +</BODY></HTML>
> diff -r 0e868f1709f1 extra/net/sourceforge/javaws/about/resources/news.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/extra/net/sourceforge/javaws/about/resources/news.html Mon Mar 07 13:26:04 2011 +0100
> @@ -0,0 +1,5 @@
> +<HTML><BODY><h1>News</h1>
> +<pre>
> +[%content%]
> +</pre>
> +</BODY></HTML>
> diff -r 0e868f1709f1 extra/net/sourceforge/javaws/about/resources/readme.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/extra/net/sourceforge/javaws/about/resources/readme.html Mon Mar 07 13:26:04 2011 +0100
> @@ -0,0 +1,5 @@
> +<HTML><BODY><h1>Readme</h1>
> +<pre>
> +[%content%]
> +</pre>
> +</BODY></HTML>
Tags should be lower-case i.e. <html> <body>. The align attribute on the image is deprecated.
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list