Reviewer needed - fix for Re: Strange behaviour during javaws
Jiri Vanek
jvanek at redhat.com
Wed Mar 9 07:16:21 PST 2011
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.
>>
>>
>> >* 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/" "/ / | 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?
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110309/926f3e02/authors.html
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110309/926f3e02/news.html
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110309/926f3e02/readme.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: javawsAuthsScreen.png
Type: image/png
Size: 62764 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110309/926f3e02/javawsAuthsScreen.png
More information about the distro-pkg-dev
mailing list