r113913 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r113912‎ | r113913 | r113914 >
Date:13:37, 15 March 2012
Author:yaron
Status:deferred (Comments)
Tags:
Comment:
Restored backward-compatibility for MW 1.17 (?), improved display of "more than one form" warning
Modified paths:
  • /trunk/extensions/SemanticForms/includes/SF_FormEditAction.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticForms/includes/SF_FormEditAction.php
——@@ -174,9 +174,18 @@
175175 if ( count( $form_names ) == 0 ) {
176176 return true;
177177 }
 178+
 179+ // For backward-compatibility
 180+ if ( is_string( $action ) ) {
 181+ global $wgOut;
 182+ $output = $wgOut;
 183+ } else {
 184+ $output = $action->getOutput();
 185+ }
 186+
178187 if ( count( $form_names ) > 1 ) {
179 - $warning_text = "\t" . '<div class="warningMessage">' . wfMsg( 'sf_formedit_morethanoneform' ) . "</div>\n";
180 - $action->getOutput()->addHTML( $warning_text );
 188+ $warning_text = "\t" . '<div class="warningbox">' . wfMsg( 'sf_formedit_morethanoneform' ) . "</div>\n";
 189+ $output->addWikiText( $warning_text );
181190 }
182191 $form_name = $form_names[0];
183192
——@@ -202,7 +211,7 @@
203212 $msg = $msg[0];
204213 }
205214
206 - $action->getOutput()->addHTML( Html::element( 'p', array( 'class' => 'error' ), wfMsg( $msg, $msgdata ) ) );
 215+ $output->addHTML( Html::element( 'p', array( 'class' => 'error' ), wfMsg( $msg, $msgdata ) ) );
207216
208217 }
209218

Comments

#Comment by Jeroen De Dauw (talk | contribs)   01:33, 20 March 2012

Ahh, that looks like an evil compat check :)

Why not do something like this, and then do $out = $this->getOutput()?

http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/Push/specials/Push_Body.php?revision=113155&view=markup#l353

And you can use the same stuff to get rid of a bunch of other globals as well :)

#Comment by Yaron Koren (talk | contribs)   01:39, 20 March 2012

Isn't that essentially the same thing? What's bad about the current code?

#Comment by Jeroen De Dauw (talk | contribs)   01:43, 20 March 2012

is_string( $action ) ???

Also, it's nicer if the only compat code you have are isolated methods that can just be dropped when applicable rather then if blocks all over the place. You'll be able to "use" the new methods even though they are not really there yet :)

#Comment by Yaron Koren (talk | contribs)   02:44, 20 March 2012

Okay - I see what you're saying, although for this case, I think SF has only one check for that method.

Status & tagging log