adding some keywords

a.d.bergi wrote:

Comment on attachment 8264
proposed patch

Index: Preprocessor_DOM.php

• Preprocessor_DOM.php (Revision 83528)

+++ Preprocessor_DOM.php (Arbeitskopie)
@@ -611,18 +611,12 @@

					$piece->count -= $matchingCount;
					# do we still qualify for any callback with remaining count?

• $names = $rules[$piece->open]['names'];
• $skippedBraces = 0;
• $enclosingAccum =& $accum;
• while ( $piece->count ) {
• if ( array_key_exists( $piece->count, $names ) ) {
• $stack->push( $piece );
• $accum =& $stack->getAccum();
• break;
• }
• --$piece->count;
• $skippedBraces ++;
• }
• $enclosingAccum .= str_repeat( $piece->open, $skippedBraces );

+ $min = $rules[$piece->open]['min'];
+ if ( $piece->count >= $names->min ) {
+ $stack->push( $piece );
+ $accum =& $stack->getAccum();
+ } else {
+ $accum .= str_repeat( $piece->open, $piece->count );
+ }

				}
				$flags = $stack->getFlags();
				extract( $flags );

Index: Preprocessor_Hash.php

• Preprocessor_Hash.php (Revision 83528)

+++ Preprocessor_Hash.php (Arbeitskopie)
@@ -624,18 +624,12 @@

					$piece->count -= $matchingCount;
					# do we still qualify for any callback with remaining count?

• $names = $rules[$piece->open]['names'];
• $skippedBraces = 0;
• $enclosingAccum =& $accum;
• while ( $piece->count ) {
• if ( array_key_exists( $piece->count, $names ) ) {
• $stack->push( $piece );
• $accum =& $stack->getAccum();
• break;
• }
• --$piece->count;
• $skippedBraces ++;
• }
• $enclosingAccum->addLiteral( str_repeat( $piece->open, $skippedBraces ) );

+ $min = $rules[$piece->open]['min'];
+ if ( $piece->count >= $min ) {
+ $stack->push( $piece );
+ $accum =& $stack->getAccum();
+ } else {
+ $accum .= str_repeat( $piece->open, $piece->count );
+ }

				}

				extract( $stack->getFlags() );

a.d.bergi wrote:

testcase

attachment bug27936_test.txt ignored as obsolete

a.d.bergi wrote:

expected testresult

attachment bug27936_test.expected ignored as obsolete

a.d.bergi wrote:

proposed patch

attachment bug27936.patch ignored as obsolete

a.d.bergi wrote:

Sorry, I didn't want to comment the patch, I tried to edit it. Forget comment 2 :-)
preprocessor-prasertests uploaded.

Could you maybe write a parserTest (phase3/tests/parser/parserTests.txt); I can't seem to understand the testcase (what is the input, what is result, what is the expected result)

a.d.bergi wrote:

I'm sorry, I don't understand the syntax of these parser tests, and http://www.mediawiki.org/wiki/Parser_tests is still a redlink.
It should be a preprocessor test, like those in phase3/tests/parser/preprocess/.

The problem is, when there are lots of opening braces, their closing does not work for more than two stack elements. You can see this at "{{{{{{ }} }} }}", which renders to "{{<tpl><tpl> </tpl> </tpl> }}".
My test is just like a cartesian product of possibilities of open and close pairs/triples, I may should shorten it.

EN.WP.ST47 wrote:

Single unified patch against phase3 with code changes and (passing!) parser test

I just took the patch and the proposed test, fixed a few whitespace issues with the test, applied the patch, set up the test, and the test is passing. This patch incorporates the three initial attachments.

attachment bug27936.patch ignored as obsolete

sumanah wrote:

Dan, thank you for the patch and the parser test! They are very much appreciated. I'm notifying Gabriel Wicke, who is working on rewriting the parser.

In the few months since you made attachment 8859, the trunk has changed a little; would you mind updating it to work against MediaWiki as it is in Subversion now? We'd appreciate that. Thank you. I'm sorry for the delay and the inconvenience.

sumanah wrote:

Comment on attachment 8859
Single unified patch against phase3 with code changes and (passing!) parser test

Per automated testing
http://lists.wikimedia.org/pipermail/wikitech-l/2011-November/056340.html patch
no longer applies to MediaWiki trunk in Subversion. Requesting update.

EN.WP.ST47 wrote:

Updated patch against r104190

What's this, Sumana, you're working on thanksgiving?

Per manual testing, the only failing hunk from the original patch had the effect of removing a single line, which by the way was already commented out. Here is the updated patch.

Attached:

sumanah wrote:

Dan, I am basically taking the long weekend off, but I find that gardening Bugzilla is a little bit calming, like how some people pop the bubbles in bubble wrap. :-)

Added the "patch" and "need-review" keywords to signal that there's a patch here that awaits review. Please do feel free to add them the next time you attach a new patch that another developer should review. Thanks for the update.

EN.WP.ST47 wrote:

See gerrit #8511

Merged -> marking as fixed.