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
solve with the static variables is a non-issue, so I've removed that
Created attachment 2541 [details]
added credits entry, so it will display better in special;version
Created attachment 2542 [details]
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
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]
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