r98986 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r98985‎ | r98986 | r98987 >
Date:08:49, 5 October 2011
Author:petrb
Status:deferred (Comments)
Tags:
Comment:
included invisible tags over diffs so that diff can be parsed easier using some scripts which use html output to get it
Modified paths:
  • /branches/petrb/phase3/includes/diff/DifferenceEngine.php (modified) (history)

Diff [purge]

Index: branches/petrb/phase3/includes/diff/DifferenceEngine.php
@@ -937,7 +937,7 @@
938938 // shared.css sets diff in interface language/dir, but the actual content
939939 // is often in a different language, mostly the page content language/dir
940940 $tableClass = 'diff diff-contentalign-' . htmlspecialchars( $this->mDiffLang->alignStart() );
941 - $header = "<table class='$tableClass'>";
 941+ $header = "<!-- diff --><table class='$tableClass'>";
942942 if ( $diff ) { // Safari/Chrome show broken output if cols not used
943943 $header .= "
944944 <col class='diff-marker' />
@@ -963,7 +963,7 @@
964964 $header .= "<tr><td colspan='{$multiColspan}' align='center'>{$notice}</td></tr>";
965965 }
966966
967 - return $header . $diff . "</table>";
 967+ return $header . $diff . "</table>i<!-- /diff -->";
968968 }
969969
970970 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r98991fixed (follow 98986)petrb10:38, 5 October 2011

Comments

#Comment by Johnduhart (talk | contribs)   10:08, 5 October 2011
return $header . $diff . "</table>i<!-- /diff -->";

What's the i in there for?

#Comment by Johnduhart (talk | contribs)   10:10, 5 October 2011

Additionally why even do this? Are diffs not available through the API?

#Comment by Petrb (talk | contribs)   10:17, 5 October 2011

I don't know at least I couldn't find it, anyway that i should have not been there, I probably just double clicked when changing mode in editor

#Comment by Petrb (talk | contribs)   11:10, 5 October 2011

I don't know at least I couldn't find it, anyway that i should have not been there, I probably just double clicked when changing mode in editor

#Comment by Petrb (talk | contribs)   11:11, 5 October 2011

eh, sorry for double post

#Comment by Petrb (talk | contribs)   09:51, 13 October 2011

can someone review it please, tyvm :)

#Comment by MaxSem (talk | contribs)   10:04, 13 October 2011

I personally see it as pointless: you can easily do $( '.diff' ).foo(), but how are you going to do it with HTML comments? The only thing this can make easier is screen-scraping by third-party tools which we actively discourage anyway.

#Comment by Petrb (talk | contribs)   11:05, 13 October 2011

you say discourage, so what you suggest? is there some way to get a diff from html output than that? like api? in that case it wouldn't be html formatted anyway

#Comment by Platonides (talk | contribs)   12:13, 13 October 2011
#Comment by Petrb (talk | contribs)   11:07, 13 October 2011

And what is problem with html comments in output which already contains html?

#Comment by MaxSem (talk | contribs)   11:24, 13 October 2011

Because it encourages screen-scraping which we fiercely discourage. Just don't do it, and you won't need to complain when we break it.

Status & tagging log