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:[EMAIL PROTECTED] > 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:[EMAIL PROTECTED] > 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: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
