You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by "Michael A. McAngus" <ma...@infinet.com> on 2002/07/14 07:57:00 UTC
Re: [SUBMIT] Timezone support for date elements of pattern layout
mwomack@apache.org wrote:
[snip]
>If you have the time and gumption, test cases to test the changes would be
>useful as well. I can help you get started on that if you need it.
>
Such a simple request; yet, because of it, I've spent much of my recent
free time learning about Ant and JUnit, and writing test cases.
I hope my current submission meets with either acceptance or rejection
(I don't know how much longer I can keep looking at these classes).
I've attached a Zip file which should stop my brain-dead email program
from adding the source files in line.
The zip contains the following modified source files:
o.a.l.helpers.AbsoluteTimeDateFormat
- Modified to use Locale-specific decimal separator between seconds
and milliseconds. This was my original intent back in late May/early June.
- Added protected method getFormatString() which returns either
"HH:mm:ss,SSS" or "HH:mm:ss.SSS" depending on the Locale-specific
decimal separator.
- Added setLocale(Locale) method.
- Added setDecimalSeparator(String) to allow the user to specify the
decimal separator.
- Added unsetDecimalSeparator() to revert decimal separator back to
Locale-specific character.
- Deprecated AbsoluteTimeDateFormat(TimeZone)
o.a.l.helpers.DateTimeDateFormat
- Modified format to match changes made in ISO8601DateFormat. Now,
these subclasses of AbsoluteTimeDateFormat create their date/time
strings in a very similar manner.
- Implemented the parse(String, ParsePosition) method to facilitate
changes required by LogFactor5 (LF5).
- Deprecated DateTimeDateFormat(TimeZone)
o.a.l.helpers.ISO8601DateFormat
- Calls the super class to get the time portion of the date/time
format that is logged, rather than copying the code from
AbsoluteTimeDateFormat.format().
- Builds the date portion of the date/time string at most output once
per day rather than at most once per second. This is not an
optimization, since the new code is slower than the old code, but it was
done to try to reduce the negative effects of the changes.
- Implemented the parse(String, ParsePosition) method to continue the
attempt to make this class as similar to DateTimeDateFormat as possible
(and because it was rather simple).
- Deprecated ISO8601DateFormat(TimeZone)
o.a.l.helpers.DateLayout
- Modified to not use the deprecated constructors in
AbsoluteTimeDateFormat and its subclasses.
- Added setDateFormat(String, String) method to round out the
collection of these methods, and rationalized how these methods deal
with null TimeZones and/or format Strings.
o.a.l.TTCCLayout
- Modified setDateFormat call in the default constructor.
o.a.l.helpers.RelativeTimeDateFormat
- Added do-nothing setTimeZone method so that the creation of all
DateFormat subclasses could follow a set sequence of method calls.
o.a.l.lf5.util.LogFileParser
- Modified to call DateTimeDateFormat.parse(String) rather than
assume that the format of the DateTimeDateFormat date is "dd MMM yyyy
HH:mm:ss,SSS". It was this code that made me realize that Pattern
Layout should have a %R option, especially since the DATE layout cannot
be relied upon to parse back into a Date object. However, I wonder if
LF5 shouldn't simply be modified to parse the XMLLayout rather than the
PatternLayout.
o.a.l.lf5.FilteredLogTableModel
- Modified getColumn(int, LogRecord) to display the date and time
String using DateTimeDateFormat.format rather than Date.toString() + the
long timestamp.
o.a.l.lf5.viewer.FilteredLogTableModel
- Modified getColumn(int, LogRecord) to display the date and time
String using DateTimeDateFormat.format rather than Date.toString() + the
long timestamp.
o.a.l.helpers.PatternParser
- Added parsing of date/time (%d) suboptions @t, @c, @l and @d.
These are the new suboptions that were prompted by Marks original post
to this thread.
- Added parsing of new PatternLayout option %R. This option simply
causes the 1-1-1970-00:00:00.000 relative timestamp to be output to the Log.
o.a.l.PatternLayout
- Modified the class JavaDocs to explain the new date/time suboptions.
The zip also contains the following new/modified Test Cases and
supporting files:
o.a.l.helpers.AbsoluteTimeDateFormatTestCase
- New JUnit TestCase for AbsoluteTimeDateFormat.
o.a.l.helpers.DateTimeDateFormatTestCase
- New JUnit TestCase for DateTimeDateFormat.
o.a.l.helpers.ISO8601DateFormatTestCase
- New JUnit TestCase for ISO8601DateFormat.
o.a.l.PatternLayoutTestCase
- Added test cases for the PatternLayout and DateFormat changes.
- Added new properties and witness files to support the new test cases.
o.a.l.util.Filter
- Modified so Time Patterns can have both types of decimal separators.
o.a.l.varia.ErrorHandlerTestCase
- Modified to use the patterns defined in Filter.
o.a.l.util.xml.DOMTestCase
- Modified to use the patterns defined in Filter.
Finally, the zip contains:
- A modified test/build.xml to run the new and modified tests.
- Unified diffs for all the modified java source.
Please let me know if there are any more modifications that need to be
made before these changes can be accepted.
--
Cheers,
Mike McAngus
mam@infinet.com
Re: [SUBMIT] Timezone support for date elements of pattern
layout
Posted by Ceki Gülcü <ce...@qos.ch>.
Mark, Mike,
I intend to release log4j 1.2.6 this week. After that I will merge the
1.2 branch with the main trunk. This is the first task to tackle
before merging Mike's changes. The 1.2-1.3 merge is likely to be time
consuming.
I believe that from the various options that Mike added only the
decimal separator and timezone should be retained.
Regarding PatternLayout enhacements, the %conversionWord{key=value,
key=value, ... } format is preferable.
The idea of searching converters by reflection is a good
idea. Regarding case sensitivity, according to the Java Beans
conventions the first letter of an option is case
insensitive while other letters are case-sensitive. For example,
helloWorld and HelloWorld are deemed equivalent while HElloWorld and
HelloWorld are not. We can follow the same convention while searching
for pattern converters.
One possible idea is for the user to give directions about the
packages to search for. For example, if the users tells use to the
search the "com.foo" package and the convention world is "%hello", we
can attempt to locate the "com.foo.HelloPatternConverter" class. We
can assume that the list of packages to search for always includes the
"org.apache.log4j.xyz" package where xyz is yet to be determined.
ps: 1.2.6 will bring enhancements to the JMSAppender and bug fixes. I
also added a getLoggerName method to LoggingEvent so that directly
accessing the categoryName is no longer needed.
At 06:44 30.07.2002 -0400, you wrote:
>mwomack@apache.org wrote:
>>Wow. Ceki, I am surprised you have not commented on this yet.
>
>Mark,
>
>Be kind. Maybe Ceki also has a real life that has kept him away from this
>list. I had a little extra time on my hands this past weekend, and I had
>been thinking about this idea since the first time Ceki had pointed me to
>his enhancements document. So I just put my current thoughts on paper.
>
>>Maybe I
>>missed it being so busy at work lately. But if Mike implements to the
>>supplied javadoc, it looks like it would implement most of the PatternLayout
>>proposal (http://qos.ch/specs/PatternLayout.html) except for maybe
>>dynamically specifiying new conversion words.
>
>That's an implementation detail. I intend to use reflection to find the
>relevant o.a.l.helpers.PatternConverter that is then added to the linked
>list. If I do this right, it should be possible to create new
>PatternConverter subclasses for new Pattern Ids, and to find these new
>PatternConverters in the Classpath. I haven't yet figured out the best
>way to find new PatternConverter subclasses (since they won't be in the
>o.a.l.helpers package), so any suggestions would be appreciated.
>
>>I would choose to have
>>lowercase conversion words instead of mixed case.
>
>That's fine. My idea is to find instantiate subclasses of
>PatternConverter using Reflection. I expect that subclasses of
>PatternConverter would be PatternidPatternConverter, so I will up shift
>the first character of the Pattern Id and down shift all the other
>characters to build the class name. Thus, Pattern Ids will not be case
>sensitive. I think that subparameters will also be case insensitive.
>
>>I would also choose the
>>single % with {} for grouping specifiers as in Ceki's proposal (instead of
>>matching % characters), but the core appears to be there.
>
>I chose beginning and ending % characters to simplify identifying the
>conversion words. As for using {} instead of @ for subparameters, I'm
>willing to do that. I have to admit, I created the Javadoc without
>reviewing Ceki's document.
>
>>But frankly, I think that doing this is not required to accept Mike's first
>>round of changes he has submitted.
>
>This would be nice. I've imported the current 1.3 code to my machine, and
>I have looked at how some of the proposed changes might affect other
>classes. For example, I think that my ideas would require changes to
>o.a.l.helpers.FormattingInfo. Since this class is only usable within its
>package (even though the class itself is public) I don't expect changes to
>it to break other code. However, I don't know if there are any
>implementations of Log4J with new classes added to the the Log4J
>packages. I don't think I've seen anything in the documentation
>discouraging creating classes in packages already found in the Log4J
>distribution.
>
>Is this a situation about which I should be concerned?
>
>--
>Cheers,
>Mike McAngus
>mam@infinet.com
>
>
>
>--
>To unsubscribe, e-mail: <ma...@jakarta.apache.org>
>For additional commands, e-mail: <ma...@jakarta.apache.org>
>
--
Ceki
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [SUBMIT] Timezone support for date elements of pattern layout
Posted by "Michael A. McAngus" <ma...@infinet.com>.
mwomack@apache.org wrote:
> Wow. Ceki, I am surprised you have not commented on this yet.
Mark,
Be kind. Maybe Ceki also has a real life that has kept him away from
this list. I had a little extra time on my hands this past weekend, and
I had been thinking about this idea since the first time Ceki had
pointed me to his enhancements document. So I just put my current
thoughts on paper.
> Maybe I
> missed it being so busy at work lately. But if Mike implements to the
> supplied javadoc, it looks like it would implement most of the PatternLayout
> proposal (http://qos.ch/specs/PatternLayout.html) except for maybe
> dynamically specifiying new conversion words.
That's an implementation detail. I intend to use reflection to find the
relevant o.a.l.helpers.PatternConverter that is then added to the linked
list. If I do this right, it should be possible to create new
PatternConverter subclasses for new Pattern Ids, and to find these new
PatternConverters in the Classpath. I haven't yet figured out the best
way to find new PatternConverter subclasses (since they won't be in the
o.a.l.helpers package), so any suggestions would be appreciated.
> I would choose to have
> lowercase conversion words instead of mixed case.
That's fine. My idea is to find instantiate subclasses of
PatternConverter using Reflection. I expect that subclasses of
PatternConverter would be PatternidPatternConverter, so I will up shift
the first character of the Pattern Id and down shift all the other
characters to build the class name. Thus, Pattern Ids will not be case
sensitive. I think that subparameters will also be case insensitive.
> I would also choose the
> single % with {} for grouping specifiers as in Ceki's proposal (instead of
> matching % characters), but the core appears to be there.
I chose beginning and ending % characters to simplify identifying the
conversion words. As for using {} instead of @ for subparameters, I'm
willing to do that. I have to admit, I created the Javadoc without
reviewing Ceki's document.
> But frankly, I think that doing this is not required to accept Mike's first
> round of changes he has submitted.
This would be nice. I've imported the current 1.3 code to my machine,
and I have looked at how some of the proposed changes might affect other
classes. For example, I think that my ideas would require changes to
o.a.l.helpers.FormattingInfo. Since this class is only usable within
its package (even though the class itself is public) I don't expect
changes to it to break other code. However, I don't know if there are
any implementations of Log4J with new classes added to the the Log4J
packages. I don't think I've seen anything in the documentation
discouraging creating classes in packages already found in the Log4J
distribution.
Is this a situation about which I should be concerned?
--
Cheers,
Mike McAngus
mam@infinet.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [SUBMIT] Timezone support for date elements of pattern layout
Posted by mw...@apache.org.
Wow. Ceki, I am surprised you have not commented on this yet. Maybe I
missed it being so busy at work lately. But if Mike implements to the
supplied javadoc, it looks like it would implement most of the PatternLayout
proposal (http://qos.ch/specs/PatternLayout.html) except for maybe
dynamically specifiying new conversion words. I would choose to have
lowercase conversion words instead of mixed case. I would also choose the
single % with {} for grouping specifiers as in Ceki's proposal (instead of
matching % characters), but the core appears to be there.
But frankly, I think that doing this is not required to accept Mike's first
round of changes he has submitted. I don't see where "%d{@lit@d}." is any
less readable than the current conversion specifiers. It is just as
cryptic, yes. Yes, I think conversion words are the way to go. But unless
there are other concrete objections/corrections to Mike's original
submission (locale, timezone, separator specifiers), I think we should
accept those changes. THEN we can look at changing
PatternLayout/PatternParser to accept conversion words, etc. Otherwise the
already large changes become even larger and harder to track. And it looks
like Mike is obviously willing to take a crack at conversion words.
I just don't think we should leave this submission hanging on the vine.
It's already taken time to just find the time to review the changes. If
there is something else that needs to be addressed, let's talk about it.
Otherwise, lets accept a very useful submission from an enthusiastic
contributor (and sign him up for something else ;-) ).
-Mark
> -----Original Message-----
> From: Michael A. McAngus [mailto:mam@infinet.com]
> Sent: Sunday, July 28, 2002 8:12 AM
> To: Log4J Developers List
> Subject: Re: [SUBMIT] Timezone support for date elements of pattern
> layout
>
>
> Ceki Gülcü wrote:
> >
> > Mike,
> >
> > It is very rare to have non-committers to submit patches of this
> > breath. Including test cases in your contribution also deserves
> > credit. I join Mark in expressing my thanks. Thank you Mike.
> >
> > As Mark observed these changes will go to 1.3 but possibly after
> > significant changes. In particular, I am uneasy about the %d suboption
> > syntax. %d{ISO8601@cCH@lit@d.@tCET} does not seem particularly user
> > friendly. Wouldn't it be possible to generalize option parsing for
> > *all* conversion specifiers?
> >
> > Please also see also http://qos.ch/specs/PatternLayout.html.
> >
>
> Since he brought up the subject twice, I decided to act on Ceki's specs
> and start working on String based pattern ids for PatternLayout (as
> opposed to the current character based pattern ids).
>
> I decided to start by writing the Javadocs that would describe the final
> set of commands, and get feedback from the rest of the list.
>
> In the process, I found some errors in the Javadocs that I submitted in
> my previous modifications to PatternLayout.
>
> Attached, please find two Zip files.
>
> _PatternLayout update.zip_ contains the updated PatternLayout, with
> corrected Javadocs, and its associated diff file.
>
> _Proposed PatternLayout.zip_ contains the Javadoc describing my proposed
> changes for PatternLayout.
>
> --
> Cheers,
> Mike McAngus
> mam@infinet.com
>
>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [SUBMIT] Timezone support for date elements of pattern layout
Posted by "Michael A. McAngus" <ma...@infinet.com>.
Ceki G�lc� wrote:
>
> Mike,
>
> It is very rare to have non-committers to submit patches of this
> breath. Including test cases in your contribution also deserves
> credit. I join Mark in expressing my thanks. Thank you Mike.
>
> As Mark observed these changes will go to 1.3 but possibly after
> significant changes. In particular, I am uneasy about the %d suboption
> syntax. %d{ISO8601@cCH@lit@d.@tCET} does not seem particularly user
> friendly. Wouldn't it be possible to generalize option parsing for
> *all* conversion specifiers?
>
> Please also see also http://qos.ch/specs/PatternLayout.html.
>
Since he brought up the subject twice, I decided to act on Ceki's specs
and start working on String based pattern ids for PatternLayout (as
opposed to the current character based pattern ids).
I decided to start by writing the Javadocs that would describe the final
set of commands, and get feedback from the rest of the list.
In the process, I found some errors in the Javadocs that I submitted in
my previous modifications to PatternLayout.
Attached, please find two Zip files.
_PatternLayout update.zip_ contains the updated PatternLayout, with
corrected Javadocs, and its associated diff file.
_Proposed PatternLayout.zip_ contains the Javadoc describing my proposed
changes for PatternLayout.
--
Cheers,
Mike McAngus
mam@infinet.com
Re: [SUBMIT] Timezone support for date elements of pattern layout
Posted by "Michael A. McAngus" <ma...@infinet.com>.
Ceki Gülcü wrote:
>
> Mike,
>
> It is very rare to have non-committers to submit patches of this
> breath. Including test cases in your contribution also deserves
> credit. I join Mark in expressing my thanks. Thank you Mike.
>
> As Mark observed these changes will go to 1.3 but possibly after
> significant changes. In particular, I am uneasy about the %d suboption
> syntax. %d{ISO8601@cCH@lit@d.@tCET} does not seem particularly user
> friendly. Wouldn't it be possible to generalize option parsing for
> *all* conversion specifiers?
>
> Please also see also http://qos.ch/specs/PatternLayout.html.
I had read this enhancement document some time ago (when you had
previously pointed me to it), and asked about backward computability issues.
Since then I have thought about enhancing PatternLayout and
PatternParser to allow for spelling out the parameter names and their
options, and using reflection to allow for new parameters with little or
no changes to PatternParser (PatternLayout's JavaDoc will always need to
be updated to reflect the new pattern elements).
I think we could also add LogLog.warn messages when the legacy pattern
elements are used, indicating that those pattern parameters are deprecated.
If I'm not required to update any of my current submissions, I could
start working on this. Tonight, I intend to pull in the 1.3 code and
see how it differs from 1.2.5.
>
> Also, have you considered the effect of contradictory options? What would
> %date{ISO8601@d!@tGMT@cUS@lit} result in (decimalSeparator=!,
> timezone=GMT, country=US, language=italian)? Log4j avoids giving
> users the possibility of specifying contradictory options.
I don't know what's contradictory about these options.
Decimal separator "!" is invalid (only "." and "," are valid until Sun
adds a new decimalSeparator to java.text.DecimalFormatSymbols). In your
example, using "@d!" has the effect of unsetting any previously set
decimal separator, so the decimal separator used would be the one
specified by Locale("it", "US"). DecimalFormatSymbols uses the Sun
defined java.text.resources.LocaleElements to determine its property
set, and given the way resource bundle lookups are done this means that
the Italian language dominates and "," (comma) is the decimal separator.
If the example were "%d{ISO8601@d.@tGMT@cUS@lit}" then setting the @d
parameter would lock in that symbol as the decimal separator until the
decimal separator is unset or changed. So, the decimal separator would
be "." regardless of the Locale.
Locales specify a country and a language. For any particular
Locale-dependent attribute, language dominates the selection of the
attribute and the choice of country may modify that attribute set.
As far as TimeZone/Locale combinations, there is nothing in the current
Java environment to prevent one from specifying any combination they
desire. Thus "@tGMT@cUS" is not contradictory.
--
Cheers,
Mike McAngus
mam@infinet.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [SUBMIT] Timezone support for date elements of pattern
layout
Posted by Ceki Gülcü <ce...@qos.ch>.
Mike,
It is very rare to have non-committers to submit patches of this
breath. Including test cases in your contribution also deserves
credit. I join Mark in expressing my thanks. Thank you Mike.
As Mark observed these changes will go to 1.3 but possibly after
significant changes. In particular, I am uneasy about the %d suboption
syntax. %d{ISO8601@cCH@lit@d.@tCET} does not seem particularly user
friendly. Wouldn't it be possible to generalize option parsing for
*all* conversion specifiers?
Please also see also http://qos.ch/specs/PatternLayout.html.
Also, have you considered the effect of contradictory options? What would
%date{ISO8601@d!@tGMT@cUS@lit} result in (decimalSeparator=!,
timezone=GMT, country=US, language=italian)? Log4j avoids giving
users the possibility of specifying contradictory options.
Best regards,
At 23:35 16.07.2002 -0700, mwomack@apache.org wrote:
>Overall, I think the changes are good. Ceki will need to review the changes
>in the various date format helper classes.
>
>What I did was check out the latest code from the main branch, copy in the
>changed files, and then reviewed them using the WinMerge tool to compare the
>files side-by-side. I successfully compiled the code. There were some
>issues with the test cases (see below).
>
>PatternLayout.java
>
>- I think the contributors list should be moved to the javadoc area so that
>deserved recognition is awarded. This is not specific to this file and has
>nothing to do with your changes. Just an observation.
>- "<li>Any pattern accpetible", should be "acceptible".
>- The private timezone member was removed. Was it not used?
>
>-----
>AbsoluteDateFormat.java
>
>- If I use the setDecimalSeparatorForLocale() method, what should be the
>value of the decSepSet member? Shouldn't it get set or reset in this
>method?
>- What is the deal with the setCalendar() method? I am a little concerned
>that we are throwing an UnsupportedOperationException. This is new?
>- I don't see where AbsoluteTimeDateFormat(TimeZone) is deprecated. It
>appears to be gone.
>
>-----
>ISO8601DateFormat.java
>
>- I don't see where ISO8601DateFormat(TimeZone) is deprecated. It appears
>to be gone.
>
>-----
>DateTimeDateFormat.java
>
>- I don't see where DateTimeDateFormat(TimeZone) is deprecated. It appears
>to be gone.
>
>-----
>PatternLayoutTestCase.java
>
>- I got the following message reported when the test case started:
>
>"D" is not a valid decimal seperator
>Using Local defined decimal seperator
>
>- Looking at test #15, the property file sets the layout to:
>
>%d{DATE@cUS@len} [%t] %-5p %.16c - %m%n
>
>but the witness file patternLayout.15 has messages that look like this:
>
> [main] DEBUG rnLayoutTestCase - Message 0
>
>Where is the date portion of the message? The witness file cannot be
>correct, can it?
>
>-----
>AbsoluteTimeDateFormatTestCase.java
>
>- I get the following error reported when the test case starts:
>
>"X" is not a valid decimal seperator
>etc
>
>-----
>tests/build.xml
>
>- Somehow the SocketServer target had an arg value changed from 4 to 5.
>This causes it to fail looking for a non-existent property file.
>
>I still need to look at the test cases some more. I will do more on this
>tomorrow.
>
>Given the level and number of changes, I think these changes should be
>applied to the 1.3 release, not the 1.2 release. I think these changes are
>good, and the submission should be accepted pending my above comments and
>the review of other committers.
>
>-Mark
--
Ceki
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [SUBMIT] Timezone support for date elements of pattern layout
Posted by "Michael A. McAngus" <ma...@infinet.com>.
mwomack@apache.org wrote:
> OK, I finally got a chance to review this latest set of changes. I am fine
> with them.
>
> My only comment is that these changes were done against the 1.2 branch. As
> such, after I have merged it into the current main branch, I get an error
> for the ErrorHandlerTestCase, which does not exist in the main branch.
>
> So, I am wondering if we should perform a merge of the 1.2 branch on to the
> main branch before applying Mike's changes. We can still keep the 1.2
> branch going after the merge?
Sorry, this is my fault. I vaguely remember this situation. In my
modified build.xml, I was comparing the list of regression tests in the
<target name="regression" ....> tag to the set of defined tests, and I
saw that the ErrorHandler test was missing. I added it in and since the
last few lines in the output say "Failures: 0, Errors: 0", I figured
that the exception was part of a successful test.
I just did a diff between my submitted build.xml and the 1.2.4
build.xml, and I see that ErrorHandler was not part of the regression
test set. Sorry for any problems/confusion.
--
Cheers,
Mike McAngus
mam@infinet.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [SUBMIT] Timezone support for date elements of pattern layout
Posted by mw...@apache.org.
OK, I finally got a chance to review this latest set of changes. I am fine
with them.
My only comment is that these changes were done against the 1.2 branch. As
such, after I have merged it into the current main branch, I get an error
for the ErrorHandlerTestCase, which does not exist in the main branch.
So, I am wondering if we should perform a merge of the 1.2 branch on to the
main branch before applying Mike's changes. We can still keep the 1.2
branch going after the merge?
-Mark
ErrorHandler:
[junit] Running org.apache.log4j.varia.ErrorHandlerTestCase
[junit] log4j:ERROR Could not open [input/xml/fallback1.xml].
[junit] java.io.FileNotFoundException: input/xml/fallback1.xml (The
system c
annot find the file specified)
[junit] at java.io.FileInputStream.open(Native Method)
[junit] at java.io.FileInputStream.<init>(FileInputStream.java:64)
[junit] at
org.apache.log4j.xml.DOMConfigurator.doConfigure(DOMConfigura
tor.java:583)
[junit] at
org.apache.log4j.xml.DOMConfigurator.configure(DOMConfigurato
r.java:694)
[junit] at
org.apache.log4j.varia.ErrorHandlerTestCase.test1(ErrorHandle
rTestCase.java:57)
[junit] at java.lang.reflect.Method.invoke(Native Method)
[junit] at junit.framework.TestCase.runTest(TestCase.java:166)
[junit] at junit.framework.TestCase.runBare(TestCase.java:140)
[junit] at junit.framework.TestResult$1.protect(TestResult.java:106)
[junit] at
junit.framework.TestResult.runProtected(TestResult.java:124)
[junit] at junit.framework.TestResult.run(TestResult.java:109)
[junit] at junit.framework.TestCase.run(TestCase.java:131)
[junit] at junit.framework.TestSuite.runTest(TestSuite.java:173)
[junit] at junit.framework.TestSuite.run(TestSuite.java:168)
[junit] at
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.
run(JUnitTestRunner.java:231)
[junit] at
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.
main(JUnitTestRunner.java:409)
[junit] log4j:WARN No appenders could be found for logger (test).
[junit] log4j:WARN Please initialize the log4j system properly.
[junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.641 sec
[junit] Testsuite: org.apache.log4j.varia.ErrorHandlerTestCase
[junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.641 sec
[junit]
[junit] Testcase: test1 took 0.641 sec
> -----Original Message-----
> From: Michael A. McAngus [mailto:mam@infinet.com]
> Sent: Wednesday, July 17, 2002 7:54 PM
> To: Log4J Developers List
> Subject: Re: [SUBMIT] Timezone support for date elements of pattern
> layout
>
>
> Michael A. McAngus wrote:
> > mwomack@apache.org wrote:
> >
> >> - "<li>Any pattern accpetible", should be "acceptible".
> >
> >
> > Actually, should be "acceptable". I'll fix this and submit a revised
> > source tonight.
>
> ...
>
> >> AbsoluteDateFormat.java
> ...
> >>
> >> - I don't see where AbsoluteTimeDateFormat(TimeZone) is deprecated. It
> >> appears to be gone.
> >>
> >> -----
> >> ISO8601DateFormat.java
> >>
> >> - I don't see where ISO8601DateFormat(TimeZone) is deprecated. It
> >> appears
> >> to be gone.
> >>
> >> -----
> >> DateTimeDateFormat.java
> >>
> >> - I don't see where DateTimeDateFormat(TimeZone) is deprecated. It
> >> appears
> >> to be gone.
> >>
> >
> > I really have no idea how that happened. I know I deprecated those
> > methods. OK, I'll add them back and deprecate them.
> >
> >> -----
> >> PatternLayoutTestCase.java
> >>
> >> - I got the following message reported when the test case started:
> >>
> >> "D" is not a valid decimal seperator
> >> Using Local defined decimal seperator
> >
> >
> > Yep. That's a successful test :-)
> > Or are you pointing out that "seperator" should be "separator"?
> Another
> > typo to fix.
> > This is a LogLog.error message letting the user know that there is a
> > problem in the configuration file.
> >
>
> ...
>
> >> -----
> >> AbsoluteTimeDateFormatTestCase.java
> >>
> >> - I get the following error reported when the test case starts:
> >>
> >> "X" is not a valid decimal seperator
> >> etc
> >
> >
> > Yep. Another successful test (except for "seperator"). Negative tests
> > are just as important as positive tests.
> >
>
> Actually, the message says "separator", so no change was needed.
>
>
> Fixes have been made, see the attached zip file containing updated
> source and new diffs.
>
> --
> Cheers,
> Mike McAngus
> mam@infinet.com
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [SUBMIT] Timezone support for date elements of pattern layout
Posted by "Michael A. McAngus" <ma...@infinet.com>.
Michael A. McAngus wrote:
> mwomack@apache.org wrote:
>
>> - "<li>Any pattern accpetible", should be "acceptible".
>
>
> Actually, should be "acceptable". I'll fix this and submit a revised
> source tonight.
...
>> AbsoluteDateFormat.java
...
>>
>> - I don't see where AbsoluteTimeDateFormat(TimeZone) is deprecated. It
>> appears to be gone.
>>
>> -----
>> ISO8601DateFormat.java
>>
>> - I don't see where ISO8601DateFormat(TimeZone) is deprecated. It
>> appears
>> to be gone.
>>
>> -----
>> DateTimeDateFormat.java
>>
>> - I don't see where DateTimeDateFormat(TimeZone) is deprecated. It
>> appears
>> to be gone.
>>
>
> I really have no idea how that happened. I know I deprecated those
> methods. OK, I'll add them back and deprecate them.
>
>> -----
>> PatternLayoutTestCase.java
>>
>> - I got the following message reported when the test case started:
>>
>> "D" is not a valid decimal seperator
>> Using Local defined decimal seperator
>
>
> Yep. That's a successful test :-)
> Or are you pointing out that "seperator" should be "separator"? Another
> typo to fix.
> This is a LogLog.error message letting the user know that there is a
> problem in the configuration file.
>
...
>> -----
>> AbsoluteTimeDateFormatTestCase.java
>>
>> - I get the following error reported when the test case starts:
>>
>> "X" is not a valid decimal seperator
>> etc
>
>
> Yep. Another successful test (except for "seperator"). Negative tests
> are just as important as positive tests.
>
Actually, the message says "separator", so no change was needed.
Fixes have been made, see the attached zip file containing updated
source and new diffs.
--
Cheers,
Mike McAngus
mam@infinet.com
Re: [SUBMIT] Timezone support for date elements of pattern layout
Posted by "Michael A. McAngus" <ma...@infinet.com>.
mwomack@apache.org wrote:
> Overall, I think the changes are good. Ceki will need to review the changes
> in the various date format helper classes.
>
> What I did was check out the latest code from the main branch, copy in the
> changed files, and then reviewed them using the WinMerge tool to compare the
> files side-by-side. I successfully compiled the code. There were some
> issues with the test cases (see below).
>
> PatternLayout.java
>
> - I think the contributors list should be moved to the javadoc area so that
> deserved recognition is awarded. This is not specific to this file and has
> nothing to do with your changes. Just an observation.
I saw it done both ways, and chose the less arrogant approach.
> - "<li>Any pattern accpetible", should be "acceptible".
Actually, should be "acceptable". I'll fix this and submit a revised
source tonight.
> - The private timezone member was removed. Was it not used?
That's correct. The private timezone String was not used.
>
> -----
> AbsoluteDateFormat.java
>
> - If I use the setDecimalSeparatorForLocale() method, what should be the
> value of the decSepSet member? Shouldn't it get set or reset in this
> method?
setDecimalSeparatorForLocale is a private method used by
unsetDecimalSeparator and setLocale. setDecimalSeparator and
unsetDecimalSeparator manage the decSepSet boolean attribute.
> - What is the deal with the setCalendar() method? I am a little concerned
> that we are throwing an UnsupportedOperationException. This is new?
We are over-riding the setCalendar method of DateFormat. As I stated in
the JavaDoc, we over-ride setCalendar because AbsoluteDateFormat is
dependent on Locale, and we can't extract the Locale from the supplied
Calendar (believe me, I tried). If a Calendar is created with a Locale
that is different from the Locale known to AbsoluteDateFormat, then we
could return unexpected results.
> - I don't see where AbsoluteTimeDateFormat(TimeZone) is deprecated. It
> appears to be gone.
>
> -----
> ISO8601DateFormat.java
>
> - I don't see where ISO8601DateFormat(TimeZone) is deprecated. It appears
> to be gone.
>
> -----
> DateTimeDateFormat.java
>
> - I don't see where DateTimeDateFormat(TimeZone) is deprecated. It appears
> to be gone.
>
I really have no idea how that happened. I know I deprecated those
methods. OK, I'll add them back and deprecate them.
> -----
> PatternLayoutTestCase.java
>
> - I got the following message reported when the test case started:
>
> "D" is not a valid decimal seperator
> Using Local defined decimal seperator
Yep. That's a successful test :-)
Or are you pointing out that "seperator" should be "separator"? Another
typo to fix.
This is a LogLog.error message letting the user know that there is a
problem in the configuration file.
>
> - Looking at test #15, the property file sets the layout to:
>
> %d{DATE@cUS@len} [%t] %-5p %.16c - %m%n
>
> but the witness file patternLayout.15 has messages that look like this:
>
> [main] DEBUG rnLayoutTestCase - Message 0
>
> Where is the date portion of the message? The witness file cannot be
> correct, can it?
For all the new date PatternLayoutTestCase tests, I used the current
functionality of that TestCase. These tests ensure that the output
matches predefined patterns. Ensuring the correctness of Date and Time
output is the responsibility of AbsoluteTimeDateFormatTestCase,
DateTimeDateFormatTestCase, and ISO8601DateFormatTestCase.
>
> -----
> AbsoluteTimeDateFormatTestCase.java
>
> - I get the following error reported when the test case starts:
>
> "X" is not a valid decimal seperator
> etc
Yep. Another successful test (except for "seperator"). Negative tests
are just as important as positive tests.
>
> -----
> tests/build.xml
>
> - Somehow the SocketServer target had an arg value changed from 4 to 5.
> This causes it to fail looking for a non-existent property file.
In the repository, the arg value is 5 for version v1_2_4. This was the
version that I coded and compared against.
>
> I still need to look at the test cases some more. I will do more on this
> tomorrow.
>
> Given the level and number of changes, I think these changes should be
> applied to the 1.3 release, not the 1.2 release. I think these changes are
> good, and the submission should be accepted pending my above comments and
> the review of other committers.
Thanks for the feedback. I'll make my updates and submit them soon
(hopefully tonight, but we are on a death march at work).
Now, I've gotta go to work.
--
Cheers,
Mike McAngus
mam@infinet.com
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: [SUBMIT] Timezone support for date elements of pattern layout
Posted by mw...@apache.org.
Overall, I think the changes are good. Ceki will need to review the changes
in the various date format helper classes.
What I did was check out the latest code from the main branch, copy in the
changed files, and then reviewed them using the WinMerge tool to compare the
files side-by-side. I successfully compiled the code. There were some
issues with the test cases (see below).
PatternLayout.java
- I think the contributors list should be moved to the javadoc area so that
deserved recognition is awarded. This is not specific to this file and has
nothing to do with your changes. Just an observation.
- "<li>Any pattern accpetible", should be "acceptible".
- The private timezone member was removed. Was it not used?
-----
AbsoluteDateFormat.java
- If I use the setDecimalSeparatorForLocale() method, what should be the
value of the decSepSet member? Shouldn't it get set or reset in this
method?
- What is the deal with the setCalendar() method? I am a little concerned
that we are throwing an UnsupportedOperationException. This is new?
- I don't see where AbsoluteTimeDateFormat(TimeZone) is deprecated. It
appears to be gone.
-----
ISO8601DateFormat.java
- I don't see where ISO8601DateFormat(TimeZone) is deprecated. It appears
to be gone.
-----
DateTimeDateFormat.java
- I don't see where DateTimeDateFormat(TimeZone) is deprecated. It appears
to be gone.
-----
PatternLayoutTestCase.java
- I got the following message reported when the test case started:
"D" is not a valid decimal seperator
Using Local defined decimal seperator
- Looking at test #15, the property file sets the layout to:
%d{DATE@cUS@len} [%t] %-5p %.16c - %m%n
but the witness file patternLayout.15 has messages that look like this:
[main] DEBUG rnLayoutTestCase - Message 0
Where is the date portion of the message? The witness file cannot be
correct, can it?
-----
AbsoluteTimeDateFormatTestCase.java
- I get the following error reported when the test case starts:
"X" is not a valid decimal seperator
etc
-----
tests/build.xml
- Somehow the SocketServer target had an arg value changed from 4 to 5.
This causes it to fail looking for a non-existent property file.
I still need to look at the test cases some more. I will do more on this
tomorrow.
Given the level and number of changes, I think these changes should be
applied to the 1.3 release, not the 1.2 release. I think these changes are
good, and the submission should be accepted pending my above comments and
the review of other committers.
-Mark
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>