Re: last Pear.php.in and DB.php commit

["Tomas V.V.Cox" <cox@idecnet.com>] > Hi Stig, > > I think there are some bugs in your last commit arround the > errorHandling stuff. I review the changes you made and made some patches > to make it work. Please review it, and commit them if are good for you > or fix your code :) > > In a quick review, I didn't find any way of using > common::setErrorHandling. Hi Tomas, Could you provide me with a quick breakage example? (The default error handling stuff was conceived in DB, so DB_common::setErrorHandling is coming of age. I'll see if it's safe to remove it.) I have one issue with your fix though... > @@ -190,7 +190,7 @@ > > function setErrorHandling($mode, $options = null) > { > - if (isset($this)) { > + /*if (isset($this)) { > $setmode = &$this->_default_error_mode; > $setoptions = &$this->_default_error_options; > $setcallback = &$this->_default_error_callback; > @@ -198,7 +198,9 @@ > $setmode = &$GLOBALS['_PEAR_default_error_mode']; > $setoptions = &$GLOBALS['_PEAR_default_error_options']; > $setcallback = &$GLOBALS['_PEAR_default_error_callback']; > - } > + }*/ > + $GLOBALS['_PEAR_default_error_mode'] = $mode; > + $GLOBALS['_PEAR_default_error_options'] = $options; Here you lose the distinction between: PEAR::setErrorHandling(PEAR_ERROR_DIE); and $obj->setErrorHandling(PEAR_ERROR_DIE); (The former variant sets the default error handling for _all_ objects, while the latter one sets it for one object.) Or did I misunderstand your fix? Here's a suggestion: modify a test (or make a new one) that demonstrates how the code is broken, and what the test results should be with correct behaviour. Communicating with code is good, and communicating with tests is better. :-) If you just send me an example, I can make the test. - Stig -- Stig Sæther Bakken <ssb@fast.no> Fast Search & Transfer ASA, Trondheim, Norway