r86072 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86071‎ | r86072 | r86073 >
Date:19:28, 14 April 2011
Author:diebuche
Status:reverted (Comments)
Tags:reedy 
Comment:
Followup to r86064 (List with double line-breaks inside tables). Also contains patch for Bug 16700 by Mormegil (Additional linebreak though nested templates). Parsertests for both
Modified paths:
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_DOM.php (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_Hash.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTests.txt (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/parser/parserTests.txt
——@@ -1435,6 +1435,37 @@
14361436
14371437 !! end
14381438 !! test
 1439+Table with broken up list inside
 1440+!! input
 1441+{|
 1442+|style="width: 5em; text-align: center"| gives
 1443+|style="border: 1px dashed #2F6FAB; padding: 0.5em; margin: 0.5em"|
 1444+# Some
 1445+# list
 1446+# Lorem
 1447+
 1448+# ipsum
 1449+# dolor
 1450+|}
 1451+!! result
 1452+<table>
 1453+<tr>
 1454+<td style="width: 5em; text-align: center">gives
 1455+</td>
 1456+<td style="border: 1px dashed #2F6FAB; padding: 0.5em; margin: 0.5em">
 1457+<ol><li> Some
 1458+</li><li> list
 1459+</li><li> Lorem
 1460+</li></ol>
 1461+<ol><li> ipsum
 1462+</li><li> dolor
 1463+</li></ol>
 1464+</td>
 1465+</tr>
 1466+</table>
 1467+
 1468+!! end
 1469+!! test
14391470 Simple paragraph
14401471 !! input
14411472 This is a simple paragraph.
——@@ -1780,6 +1811,8 @@
17811812 <td>1
17821813 </td>
17831814 <td>2
 1815+<p><br />
 1816+</p>
17841817 </td>
17851818 </tr>
17861819 <tr>
——@@ -3374,7 +3407,36 @@
33753408 !! result
33763409 ==Section 1==
33773410 !! end
 3411+!! article
 3412+Template:Top-level template
 3413+!! text
 3414+{{Nested template}}
 3415+!! endarticle
33783416
 3417+!! article
 3418+Template:Nested template
 3419+!! text
 3420+*Item 1
 3421+*Item 2
 3422+!! endarticle
 3423+
 3424+!! test
 3425+Line-start flag in a nested template call
 3426+!! input
 3427+*Item A
 3428+*Item B
 3429+
 3430+{{Top-level template}}
 3431+!! result
 3432+<ul><li>Item A
 3433+</li><li>Item B
 3434+</li></ul>
 3435+<ul><li>Item 1
 3436+</li><li>Item 2
 3437+</li></ul>
 3438+
 3439+!! end
 3440+
33793441 ###
33803442 ### Pre-save transform tests
33813443 ###
——@@ -5928,6 +5990,7 @@
59295991
59305992 !!result
59315993 <p><a rel="nofollow" class="external free" href="http://===r:::https://b">http://===r:::https://b</a>
 5994+</p><p><br />
59325995 </p>
59335996 !! end
59345997
——@@ -5966,7 +6029,7 @@
59676030 <p>{{{|
59686031 <u class="&#124;">}}}} &gt;
59696032 <br style="onmouseover=&#39;alert(document.cookie);&#39;" />
5970 -MOVE YOUR MOUSE CURSOR OVER THIS TEXT
 6033+</p><p>MOVE YOUR MOUSE CURSOR OVER THIS TEXT
59716034 </p>
59726035 <table>
59736036 <tr>
Index: trunk/phase3/includes/parser/Preprocessor_DOM.php
——@@ -521,7 +521,7 @@
522522 'open' => $curChar,
523523 'close' => $rule['end'],
524524 'count' => $count,
525 - 'lineStart' => ($i > 0 && $text[$i-1] == "\n"),
 525+ 'lineStart' => ($i == 0 || $text[$i-1] == "\n"),
526526 );
527527
528528 $stack->push( $piece );
Index: trunk/phase3/includes/parser/Preprocessor_Hash.php
——@@ -493,7 +493,7 @@
494494 'open' => $curChar,
495495 'close' => $rule['end'],
496496 'count' => $count,
497 - 'lineStart' => ($i > 0 && $text[$i-1] == "\n"),
 497+ 'lineStart' => ($i == 0 || $text[$i-1] == "\n"),
498498 );
499499
500500 $stack->push( $piece );
Index: trunk/phase3/includes/parser/Parser.php
——@@ -831,11 +831,8 @@
832832
833833 # empty line, go to next line,
834834 # but only append \n if outside of table
835 - if ( $line === '' ) {
836 - $out .= $outLine;
837 - if ( !isset( $tables[0] ) ) {
838 - $out .= "\n";
839 - }
 835+ if ( $line === '') {
 836+ $output .= $outLine . "\n";
840837 continue;
841838 }
842839 $firstChars = $line[0];

Follow-up revisions

RevisionCommit summaryAuthorDate
r86419Change tests to match preprocessor changes in r86072....platonides18:39, 19 April 2011
r87211Update to incorporate the Preprocessor change of r86072 of what is a lineStartplatonides19:39, 1 May 2011
r87214Follow up r87211, update the unserializeNode() and fix flag resetting which w...platonides20:35, 1 May 2011
r96887Reverted r86072, r86419 per CR. Lots of conflicts resolved here. Removes line...aaron19:16, 12 September 2011
r96906Updated r86144 code due to reverts in r96889 (specifically of Parser.php chan...aaron21:15, 12 September 2011
r101283Revert r87211 as r86072 which it was mimicking was also reverted in r96887....platonides21:13, 29 October 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86064Fix for r85990, which broke lists inside tables; adding test for itdiebuche18:45, 14 April 2011

Comments

#Comment by Platonides (talk | contribs)   22:10, 18 April 2011

Doesn't update PreprocessorTests, and so breaks 28 of them.

I was wary of sideffects, but after looking for problems arising from things like bug 529 or bug 12974, seems right.

#Comment by DieBuche (talk | contribs)   09:20, 19 April 2011

Could you or someone else fix those? I can't seem to get phpunit running

#Comment by Platonides (talk | contribs)   18:41, 19 April 2011

Fixed them in r86419.

#Comment by Bawolff (talk | contribs)   23:57, 16 August 2011

I'm marking fixme because its causing bug 30384 which seems like it has the potential to add lots of excess newlines to pre-existing templates

#Comment by Reedy (talk | contribs)   18:49, 24 August 2011

Bawolff, can you verify the patch in https://bugzilla.wikimedia.org/attachment.cgi?id=8931&action=diff

As if it doesn't fix it, I'm going to revert this revision and then merge out of 1.18

Cheers!

#Comment by Bawolff (talk | contribs)   04:06, 25 August 2011

The patch on that bug is essentially just reverting this revision. It will fix DaSch's test case, and break about 6 parserTests from what I can tell (which are probably the one's changed in this revision)

To be honest, what this revision is doing is a bit above my head.

Status & tagging log