ssanbeg wrote:

fix for 1.8

attachment poem-nw.patch ignored as obsolete

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

ssanbeg wrote:

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

attachment poemParserTests.txt ignored as obsolete

ssanbeg wrote:

fix problems with previous versions

enhancement to fix problem with recursive text in pre 1.8; preven possible
bloating of strip state.

attachment poem-nw.patch ignored as obsolete

ssanbeg wrote:

add regression case for previous patch

attachment poemParserTests.txt ignored as obsolete

ssanbeg wrote:

minor format changes, add one more test case

This should be the last change for awhile...

attachment poemParserTests.txt ignored as obsolete

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.

ssanbeg wrote:

supress sputious error in previous version

attachment poem-nw.patch ignored as obsolete

ssanbeg wrote:

(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.

ssanbeg wrote:

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.

ssanbeg wrote:

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.

attachment poem-nw-np.patch ignored as obsolete

ssanbeg wrote:

added credits entry, so it will display better in special;version

attachment poem.patch ignored as obsolete

ssanbeg wrote:

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.

attachment poemParserTests.txt ignored as obsolete

ssanbeg wrote:

new patch, with fix for paragraph with leading space.

attachment poem.patch ignored as obsolete

ssanbeg wrote:

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.

attachment LabeledSectionTransclusion.php ignored as obsolete

ssanbeg wrote:

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.

attachment lst-tests.txt ignored as obsolete

ssanbeg wrote:

Sorry, those last two attachments were misfiled, please disregard.

ssanbeg wrote:

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.

attachment poem.patch ignored as obsolete

ssanbeg wrote:

corrected patch.

Attached:

ssanbeg wrote:

update regression tests to work with Simetrical's changes

Attached:

ssanbeg wrote:

This has been stable for awhile, so I may as well resolve it. Committed in r18340