Re: BBCodeParser (CVS commit(s))

It looks fine with me. Test cases still work... Some of your commenting changes, although harmless, seemed unnecessary. Other commenting changes (using // with multiline comments) go against what I have heard is good style. ~Seth On Oct 17, 2005, at 4:54 AM, Lorenzo Alberton wrote:

Hi Seth, guys,

I attached my testcases to the end of my last email, but I don't think that anyone noticed them. This time I've zipped them and put them on my website: http://pricepages.org/bbcode/BBCodeParser.phpt.zip They require PHPUnit. You can run the test cases by running the script from the command line ("php -f BBCodeParser.phpt"). I was also asked to look at the patch here: http://news.php.net/php.pear.cvs/35523 I applied the patch to the latest CVS sources and it still passes my original testcases,

confirmed: TestCase bbcodeparser_testcase->testqparse() passed BTW: thanks a lot for the testsuite!

The removeFilter() bug occurs because the tags need to be removed from $_definedTags in addition to the filter being removed from $_filters.

good point ;)

I'll let you fix that one :).

done.

Please update your patch to include fixes for the two problems that I found.

with the new patch [1]: TestCase bbcodeparser_testcase->testfilters() passed TestCase bbcodeparser_testcase->testqparse() passed Can I commit it, now? Best regards, --Lorenzo Alberton http://pear.php.net/user/quipo [1] http://www.archaeogate.org/tmp/pear/BBCodeParser.patch.txt --PEAR Development Mailing List (http://pear.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php