Reviewer needed - fix for Re: Strange behaviour during javaws

Thu Mar 10 03:22:44 PST 2011

On 03/09/2011 04:16 PM, Jiri Vanek wrote:
> On 03/07/2011 07:06 PM, Dr Andrew John Hughes wrote:
>> 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?
>>
>
> Don't agree. Most of the links is not working. If we want some demo we
> can add some working and useful (eg text editor (one of working) and
> sweet home.
>
>>>
>>> >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.
>
> Ugh. Dont you think it quite drastic? Can you consider disgusting
> reading of something like
> <authors>
> <author> <name>xp13</name><email>xp13 at gm.com</email></author></authors>
> ? I can not. Same for news. It is palintext, every one is used for it
> and it should remain plaintext.
> And it will add complex dependence of xalan or some another xslt processor.
>>
>>> 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.
>>
> work in progress.

hmmm... I still thing it is the best place. It is the time when all 
classes are compiled and resources copied/prepared.

>>>
>>>
>>> >* 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?
>
> Yes, yes, as mentioned. It is here (in repository) very very long time.
> (since original netx)
>>
>>>
>>> >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 consider netbeans as best java editor (maybe except JIdea)! Just
> friendly quizzical question? Which one you recommend? (And dont mention
> ((g)vim! :D )
>>
>>>
>>> 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?
>
> Htmls and screenshots attached;)
>>
>>> 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.
>
> Yes i know. (with above, working on it). But when i move generation
> elsewhere it will cause some more fixes upon copiing another resources.
>
>> You shouldn't use 'java' but $(JAVA).
>
> ok
>
>> 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.
>
> It is compiled with rest of about package.
> Installation is discussable, but i can remove it form it.
>
>>
>>> 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.
>
> sure. deleted.
>>
>>> +
>>> +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/" "/&nbsp;/ | 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&nbsp; ");
>>> + 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="&nbsp;";
>>> + }
>>> + 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?
>
> Till java do not heave automatic accessors - then yes.
>>
>>> +
>>> + 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.
>
> No no - this method request all-permissions tag.
>>
>>>
>>> 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.
>
>
> Do you really care about lowercase/uppercase tags? But as you command.
> Centre - yap - inherited typo. Moreover it is useless in this situation.
>
>>
>
>
> Regards J.