DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger --- Additional Comments From [EMAIL PROTECTED] 2004-09-12 18:22 --- Q 2.) Having the constructor for CommandLineOptions take the args[] parameter again. I don't like the ability to create objects in an invalid state--in this case, having a CommandLineOptions object alive without having a command-line options string. So I tend to prefer constructors that require what is needed up-front--it saves validation code later. /Q Sure, but then we need some other way of logging the exceptions thrown during command argument parsing. Right now we get can get an exception during the CommandLineOptions ctor. As a result the options vrbl is null in Fop.main() and we are not able to output the exception to the logger which was created but lost during the exception. Try f.ex: fop -fo -pdf out to see that no information about the exception is printed. regards, finn
DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger --- Additional Comments From [EMAIL PROTECTED] 2004-09-13 03:22 --- OK, please fix it then. Xalan [1] doesn't bother with throwing exceptions during command line processing, they just System.err the message and then call System.exit(). I'd like us to move in that direction, it would also sharply reduce the number of FOPExceptions we have. WDYT? (Another pet project of mine, to reduce FOPException in favor of what the actual exception is. BTW, anyone would object to us eventually getting rid of FOPException entirely? I think it breeds bad coding habits, because the developer doesn't stop to think what the actual exception type is.) [1] http://cvs.apache.org/viewcvs.cgi/xml-xalan/java/src/org/apache/xalan/xslt/Process.java?rev=1.62view=auto Thanks, Glen
DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger --- Additional Comments From [EMAIL PROTECTED] 2004-09-11 16:43 --- Oh, so this is what Jeremias meant. ;) Nice implementation, but would like two changes to your patch: 1.) I would like CommandLineLogger to be in the apps package, because I prefer the packages to be more standalone and separable from each other. CLL is only used by CommandLineOptions, so apps IMO is best. 2.) Having the constructor for CommandLineOptions take the args[] parameter again. I don't like the ability to create objects in an invalid state--in this case, having a CommandLineOptions object alive without having a command-line options string. So I tend to prefer constructors that require what is needed up-front--it saves validation code later. Otherwise, all looks good. +1. Glen
DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |ASSIGNED --- Additional Comments From [EMAIL PROTECTED] 2004-09-11 17:08 --- Thanks, Finn. Exactly what I meant. Glen, would you mind leaving CommandLineLogger in the util package? It's because we've got at least two other command line applications (PFMReader and TTFReader) which could use this class. But I agree with your other point. Just FYI, I've applied Finn's patch locally and have also made a few trivial modifications (mostly style and javadocs, only the \n should have been within the if). I will also do the necessary changes for PFMReader and TTFReader. I can commit the whole thing quickly.
DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger --- Additional Comments From [EMAIL PROTECTED] 2004-09-11 18:22 --- Q Glen, would you mind leaving CommandLineLogger in the util package? It's because we've got at least two other command line applications (PFMReader and TTFReader) which could use this class. But I agree with your other point. /Q OK.
DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger [EMAIL PROTECTED] changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Additional Comments From [EMAIL PROTECTED] 2004-09-11 18:59 --- Applied.
DO NOT REPLY [Bug 31162] - [PATCH] Commandline logger
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT http://issues.apache.org/bugzilla/show_bug.cgi?id=31162. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=31162 [PATCH] Commandline logger --- Additional Comments From [EMAIL PROTECTED] 2004-09-10 13:42 --- Created an attachment (id=12692) Unified patch against HEAD.
