logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul Smith" <psm...@aconex.com>
Subject RE: cvs commit: logging-log4j/src/java/org/apache/log4j/chainsaw/help release-notes.html
Date Mon, 02 Aug 2004 05:46:21 GMT
Ahhhhhhhhhh.  All makes sense now.   Perhaps we should leave ourselves a
note in the code then to highlight the 'weirdness' of why we don't notify
listeners at this point.   (Maybe a TODO while we're there).

Thanks for taking the time to write a really detailed description.

Cheers,

Paul

> -----Original Message-----
> From: Scott Deboy [mailto:sdeboy@comotivsystems.com]
> Sent: Monday, August 02, 2004 3:40 PM
> To: Log4J Developers List
> Subject: RE: cvs commit: logging-
> log4j/src/java/org/apache/log4j/chainsaw/help release-notes.html
> 
> The code calling clear (ColorPanel's applyRules method) does end up
> triggering the colorrule propertychange notification, through the
> 'addRules' call (as does setRules).  Clear is unneeded - I'll change the
> caller to use setRules and remove the clear method.
> 
> There was a cycle in the ColorPanel/Colorizer interaction causing the
> removal of all color rules when you hit the 'apply' button.
> 
> The cause:
> ColorPanel is registered as a propertyChangeListener on colorizer for
> colorrule events.  This listener calls updateColors, which clears
> ColorPanel's tableModel and repopulates it based on Colorizer's rules.
> 
> ColorPanel's applyRules method begins by calling colorizer.clear, which
> clears it's events and triggers the callback on ColorPanel.  This callback
> (updateColors) results in no color rules being saved.
> 
> Removing the clear method altogether seems like the right thing to do,
> since applyRules can call 'setRules' (which does notify any property
> change listeners of the colorrule update).
> 
> It's confusing, but the code is confusing.  Colorizer/ColorPanel/LogPanel
> interactions should probably be examined carefully.  This isn't an ideal
> fix but it prevents the regression of 'apply' deleting all of the color
> rules.
> 
> -----Original Message-----
> From:	Paul Smith [mailto:psmith@aconex.com]
> Sent:	Sun 8/1/2004 3:10 PM
> To:	'Log4J Developers List'
> Cc:
> Subject:	RE: cvs commit: logging-
> log4j/src/java/org/apache/log4j/chainsaw/help release-notes.html
> I'm curious about the below change.  I would have thought the code should
> notify listeners when the information the rule is based on is changed?
> Could you describe why we need to remove that line?
> 
> Paul
> >
> >   Index: RuleColorizer.java
> >   ===================================================================
> >   RCS file: /home/cvs/logging-
> > log4j/src/java/org/apache/log4j/chainsaw/color/RuleColorizer.java,v
> >   retrieving revision 1.9
> >   retrieving revision 1.10
> >   diff -u -r1.9 -r1.10
> >   --- RuleColorizer.java	28 Jul 2004 08:02:17 -0000	1.9
> >   +++ RuleColorizer.java	31 Jul 2004 07:23:58 -0000	1.10
> >   @@ -117,7 +117,6 @@
> >
> >      public void clear() {
> >        rules.clear();
> >   -    colorChangeSupport.firePropertyChange("colorrule", false, true);
> >      }
> >
> >      public void removeRule(String ruleSetName, String expression) {
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message