Re: [cp-patches] FYI: gjar @file fix
2008/6/3 Robert Schuster <[EMAIL PROTECTED]>: > Hi Tom, > >> I am less sure about adding java-specific file name parsing to the >> generic command-line parser code. That code is not supposed to be >> specific to java tooling (at least frysk uses it, fwiw). > I will implement the @file handling ClasspathToolParser now. That way > only our Classpath tools are affected. > > Regards > Robert > > That seems a good option. Thanks again for doing this. Let me know before you commit, and I'll revert my gjar patch if that makes things easier. -- Andrew :-) Support Free Java! Contribute to GNU Classpath and the OpenJDK http://www.gnu.org/software/classpath http://openjdk.java.net PGP Key: 94EFD9D8 (http://subkeys.pgp.net) Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
Re: [cp-patches] FYI: gjar @file fix
Hi Tom, > I am less sure about adding java-specific file name parsing to the > generic command-line parser code. That code is not supposed to be > specific to java tooling (at least frysk uses it, fwiw). I will implement the @file handling ClasspathToolParser now. That way only our Classpath tools are affected. Regards Robert signature.asc Description: OpenPGP digital signature
Re: [cp-patches] FYI: gjar @file fix
> "Robert" == Robert Schuster <[EMAIL PROTECTED]> writes: Robert> What my implementation does not handle is whitespace inside filenames Robert> and quoting. However I am not sure whether this is supported in filelist Robert> anyway. Somewhere there is a rule for how javac handles this. As I recall, jacks had a test for it; this is what I used when writing gcjx :) In any case, it is easy enough to test the JDK jar to see what it does. Robert> It may be possible that some tools need @file support and some Robert> not (Does anyone know more about this?). And such a case I Robert> would change the getopt API to explicitly request @file Robert> support in parse(). I think it is fine to add some code to the generic parser to let subclasses treat '@' specially, if that is needed. I am less sure about adding java-specific file name parsing to the generic command-line parser code. That code is not supposed to be specific to java tooling (at least frysk uses it, fwiw). Tom
Re: [cp-patches] FYI: gjar @file fix
Hi Andrew, >> I was trying to get Classpath to build PhoneME today and that requires >> our javah to support @-style arguments as well. From my work for MIDPath >> I know that we also lack that functionality in gjar that is why I tried >> an implementation that can be used in all getopt using classpath tools. >> > > Ok I wasn't aware you were working on this, sorry. Sorry, I did not announce this anywhere. :) >> What I am also unhappy is that a simple BufferedReader.readLine() is >> used to get the filelist entries. The filelist the phoneme build >> generates looks like this: "Bla Foo Baz". So a simple parser is needed >> that splits at whitespace bounds. My suggested implementation does >> exactly that. >> > > So I see. Is there a reason you didn't use String.split or > StringTokenizer for this? I considered String.split() to be to costly (regular expressions). StringTokenizer would be a good idea ... > The patch looks > good, though I wonder how we can still integrate the stdin support > gjar has. I just saw that StreamTokenizer might provide a good parser that can also deal with quoting. If we use that in the parser and add an optional InputStream argument to Parser.parse() we could have the best of your and my approach and additionally get quoting right. Unfortunately I need some sleep now and will continue with this patch in ~10 hours. Regards Robert signature.asc Description: OpenPGP digital signature
Re: [cp-patches] FYI: gjar @file fix
> > Andrew John Hughes schrieb: > > > This adds support in gjar for the @file argument > > as used by the OpenJDK build. > > > > ChangeLog: > > > > 2008-06-02 Andrew John Hughes <[EMAIL PROTECTED]> > > > > * tools/gnu/classpath/tools/getopt/OptionException.java: > > (OptionException(String,Throwable)): New constructor. > > * tools/gnu/classpath/tools/jar/Main.java: > > (fileLists): New queue for streams containing lists of files. > > (HandleFile.NotifyFile(String)): Check for '@' arguments > > and add to stream queue. > > (parsed(String)): Add stdin to queue instead of setting flag. > > (readNames()): Work with the queue rather than just stdin. > > (run(String[])): Always execute readNames(). > > > > > > > On 02/06/2008, Robert Schuster <[EMAIL PROTECTED]> wrote: > Hi Andrew, > I am not so happy with that patch. :) > > I was trying to get Classpath to build PhoneME today and that requires > our javah to support @-style arguments as well. From my work for MIDPath > I know that we also lack that functionality in gjar that is why I tried > an implementation that can be used in all getopt using classpath tools. > Ok I wasn't aware you were working on this, sorry. The patch looks good, though I wonder how we can still integrate the stdin support gjar has. > What I am also unhappy is that a simple BufferedReader.readLine() is > used to get the filelist entries. The filelist the phoneme build > generates looks like this: "Bla Foo Baz". So a simple parser is needed > that splits at whitespace bounds. My suggested implementation does > exactly that. > So I see. Is there a reason you didn't use String.split or StringTokenizer for this? > What my implementation does not handle is whitespace inside filenames > and quoting. However I am not sure whether this is supported in filelist > anyway. > > It may be possible that some tools need @file support and some not (Does > anyone know more about this?). And such a case I would change the getopt > API to explicitly request @file support in parse(). > > Please have a look at the attached patch. :) > > Regards > Robert Thanks, -- Andrew :-) Support Free Java! Contribute to GNU Classpath and the OpenJDK http://www.gnu.org/software/classpath http://openjdk.java.net PGP Key: 94EFD9D8 (http://subkeys.pgp.net) Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
Re: [cp-patches] FYI: gjar @file fix
Hi Andrew,
I am not so happy with that patch. :)
I was trying to get Classpath to build PhoneME today and that requires
our javah to support @-style arguments as well. From my work for MIDPath
I know that we also lack that functionality in gjar that is why I tried
an implementation that can be used in all getopt using classpath tools.
What I am also unhappy is that a simple BufferedReader.readLine() is
used to get the filelist entries. The filelist the phoneme build
generates looks like this: "Bla Foo Baz". So a simple parser is needed
that splits at whitespace bounds. My suggested implementation does
exactly that.
What my implementation does not handle is whitespace inside filenames
and quoting. However I am not sure whether this is supported in filelist
anyway.
It may be possible that some tools need @file support and some not (Does
anyone know more about this?). And such a case I would change the getopt
API to explicitly request @file support in parse().
Please have a look at the attached patch. :)
Regards
Robert
Andrew John Hughes schrieb:
> This adds support in gjar for the @file argument
> as used by the OpenJDK build.
>
> ChangeLog:
>
> 2008-06-02 Andrew John Hughes <[EMAIL PROTECTED]>
>
> * tools/gnu/classpath/tools/getopt/OptionException.java:
> (OptionException(String,Throwable)): New constructor.
> * tools/gnu/classpath/tools/jar/Main.java:
> (fileLists): New queue for streams containing lists of files.
> (HandleFile.NotifyFile(String)): Check for '@' arguments
> and add to stream queue.
> (parsed(String)): Add stdin to queue instead of setting flag.
> (readNames()): Work with the queue rather than just stdin.
> (run(String[])): Always execute readNames().
>
>
? sun/security
? tools/generated
Index: ChangeLog
===
RCS file: /sources/classpath/classpath/ChangeLog,v
retrieving revision 1.9623
diff -u -r1.9623 ChangeLog
--- ChangeLog 1 Jun 2008 12:01:11 - 1.9623
+++ ChangeLog 2 Jun 2008 22:40:10 -
@@ -1,3 +1,10 @@
+2008-06-03 Robert Schuster <[EMAIL PROTECTED]>
+
+ * tools/gnu/classpath/tools/getopt/Parser.java:
+ (parse): Added twos checks for '@' character and calls to parseFileList.
+ (parseFileList): New method.
+ (parseLine): New method.
+
2008-06-01 Mark Wielaard <[EMAIL PROTECTED]>
* gnu/java/awt/java2d/AbstractGraphics2D.java: Removed XDialogPeer
Index: tools/gnu/classpath/tools/getopt/Parser.java
===
RCS file: /sources/classpath/classpath/tools/gnu/classpath/tools/getopt/Parser.java,v
retrieving revision 1.10
diff -u -r1.10 Parser.java
--- tools/gnu/classpath/tools/getopt/Parser.java 20 Mar 2008 18:04:44 - 1.10
+++ tools/gnu/classpath/tools/getopt/Parser.java 2 Jun 2008 22:40:11 -
@@ -38,6 +38,10 @@
package gnu.classpath.tools.getopt;
+import java.io.BufferedReader;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
import java.io.PrintStream;
import java.text.BreakIterator;
import java.text.MessageFormat;
@@ -442,7 +446,12 @@
|| args[currentIndex].charAt(0) != '-'
|| "-".equals(args[currentIndex])) //$NON-NLS-1$
{
-files.notifyFile(args[currentIndex]);
+// Treat file names starting with @ like a file containing a file list.
+ if (args[currentIndex].codePointAt(0) == '@')
+ parseFileList(args[currentIndex].substring(1), files);
+ else
+ files.notifyFile(args[currentIndex]);
+
continue;
}
if ("--".equals(args[currentIndex])) //$NON-NLS-1$
@@ -456,7 +465,13 @@
}
// Add remaining arguments to leftovers.
for (++currentIndex; currentIndex < args.length; ++currentIndex)
- files.notifyFile(args[currentIndex]);
+ {
+ // Treat file names starting with @ like a file containing a file list.
+ if (args[currentIndex].codePointAt(0) == '@')
+ parseFileList(args[currentIndex].substring(1), files);
+ else
+ files.notifyFile(args[currentIndex]);
+ }
// See if something went wrong.
validate();
}
@@ -492,4 +507,71 @@
});
return (String[]) fileResult.toArray(new String[0]);
}
+
+ /** Simple function that takes the given file, treats it as a textfile
+ * and reads all the whitespace separated entries from it notifying
+ * [EMAIL PROTECTED] FileArgumentCallback} instance each time.
+ */
+ private void parseFileList(String fileName, FileArgumentCallback files)
+throws OptionException
+ {
+BufferedReader reader = null;
+String line = null;
+
+try
+ {
+reader = new BufferedReader(new FileReader(fileName));
+ }
+catch (FileNotFoundException fnfe)
+ {
+
