Last modified: 2006-12-14 21:43:31 UTC
text in <poem><nowiki> gets a <br> rendered as text at the end of each line; using new hooks in 1.8 can prevent that.
Created attachment 2461 [details] fix for 1.8
Can you provide some test cases to confirm the fix and detect regressions? See extensions/Cite/citeParserTests.txt for an example; run them with maintenance/parserTest.php --file=etc
Created attachment 2462 [details] requested test cases 1 - simple regerssion test for <poem>, for regression -w- current version 2 - example of <nowiki>, for context 3 - nested <poem><nowiki>, to exercise new feature
Created attachment 2464 [details] fix problems with previous versions enhancement to fix problem with recursive text in pre 1.8; preven possible bloating of strip state.
Created attachment 2465 [details] add regression case for previous patch
Created attachment 2466 [details] minor format changes, add one more test case This should be the last change for awhile...
This code generates PHP notices when run, indicating some sloppy checks: Notice: Undefined index: general in /opt/web/pages/trunk/extensions/Poem/Poem.php on line 37 Always test code with error_reporting set to E_ALL. I'm also a bit uneasy about these static variable; it looks like after the first <poem> tag, all others will behave differently because $tag is now set.
Created attachment 2491 [details] supress sputious error in previous version
(In reply to comment #7) > This code generates PHP notices when run, indicating some sloppy checks: > > Notice: Undefined index: general in /opt/web/pages/trunk/extensions/Poem/Poem.php on line 37 > > Always test code with error_reporting set to E_ALL. > OK, thanks. That message is harmless; I've updated the patch to fix it. > > I'm also a bit uneasy about these static variable; it looks like after the > first <poem> tag, all others will behave differently because $tag is now set. There is enough checking there that it shouldn't be a problem. Originally, the function just added a tag ID, but I was concerned that the strip state would grow if every poem tag added to it, so I added the static variable, to remember if the tag was already inserted. If it has it, it verifies that it still has the correct value (this is where the warning was produced, so I added the check that it's still there and the same value). It it's already there, then we can just reuse the old value, rather than duplicate it. But if the check fails, (say, something else changed or removed its entry) then we can ignore the old value, and reinitialize it. From the testing I've done so far, this seems like the best way to do it; unless there's something else for working with the strip state that I haven't noticed.
Well, why would you maintain this across separate calls which can be on separate Parser instances? It just doesn't many any sense to have a static variable.
I'm expecting that adding to the strip state on each call may effectively create a memory leak. Do you mean that clearState() is called often enough that anything I add will be short lived, anyway? If so, that would simplify things.
Created attachment 2496 [details] new patch, no static vars. It appears that clearState() is called frequently enough that what I was trying to solve with the static variables is a non-issue, so I've removed that functionality.
Created attachment 2541 [details] added credits entry, so it will display better in special;version
Created attachment 2542 [details] regression tests Added another test case, to test paragraphs with leading space. If nothing else, at least a lot of useful regression tests are being found, bot for old version and new enhancement.
Created attachment 2543 [details] new patch, with fix for paragraph with leading space.
Created attachment 2629 [details] Reimplemented as an extension This extension adds the <section> hook, and two parser functions, #lst to include a section, and #lstx to exclude. It only needs to be included from LocalSettings.php to be used.
Created attachment 2630 [details] regression tests for the extension These are the regression tests for this extension, and show most of its functionality. Note that this depends on bug 7801 to run the regression test, since parser functions currently can't be regression tested.
Sorry, those last two attachments were misfiled, please disregard.
Created attachment 2760 [details] updated patch to work with Rob's changes, etc. Update for new version. Add Brion as author, per Nikola's request. Use new 1.9 hook to run tests automatically. Use method_exists instead of version_compare to detect version.
Created attachment 2761 [details] corrected patch.
Created attachment 2762 [details] update regression tests to work with Simetrical's changes
This has been stable for awhile, so I may as well resolve it. Committed in r18340
