r70540 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r70539‎ | r70540 | r70541 >
Date:05:47, 6 August 2010
Author:yaron
Status:deferred (Comments)
Tags:
Comment:
Added strReplaceOnce() utility function, changed one str_replace() call to use strReplaceOnce() instead
Modified paths:
  • /trunk/extensions/SemanticForms/includes/SF_FormPrinter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticForms/includes/SF_FormPrinter.php
@@ -124,6 +124,25 @@
125125 return false;
126126 }
127127
 128+ /**
 129+ * Like PHP's str_replace(), but only replaces the first found instance -
 130+ * unfortunately, str_replace() doesn't allow for that.
 131+ * This code is basically copied directly from
 132+ * http://www.php.net/manual/en/function.str-replace.php#86177
 133+ * - this might make sense in some SF utils class, if it's useful in
 134+ * other places.
 135+ */
 136+ function strReplaceOnce( $search, $replace, $subject) {
 137+ $firstChar = strpos( $subject, $search );
 138+ if ( $firstChar !== false ) {
 139+ $beforeStr = substr( $subject, 0, $firstChar );
 140+ $afterStr = substr( $subject, $firstChar + strlen( $search ) );
 141+ return $beforeStr . $replace . $afterStr;
 142+ } else {
 143+ return $subject;
 144+ }
 145+ }
 146+
128147 function formHTML( $form_def, $form_submitted, $source_is_page, $form_id = null, $existing_page_content = null, $page_name = null, $page_name_formula = null, $is_query = false, $embedded = false ) {
129148 global $wgRequest, $wgUser, $wgParser;
130149 global $sfgTabIndex; // used to represent the current tab index in the form
@@ -421,7 +440,7 @@
422441 $existing_page_content = str_replace( $existing_template_text, '{{{insertionpoint}}}', $existing_page_content );
423442 }
424443 } else {
425 - $existing_page_content = str_replace( $existing_template_text, '', $existing_page_content );
 444+ $existing_page_content = self::strReplaceOnce( $existing_template_text, '', $existing_page_content );
426445 }
427446 // if this is not a multiple-instance template, and we've found
428447 // a match in the source page, there's a good chance that this

Comments

#Comment by Jeroen De Dauw (talk | contribs)   12:50, 19 August 2010

What about naming this strReplaceFirst? The 'once' in the current name is more appropriate for indicating a single iteration.

#Comment by Yaron Koren (talk | contribs)   17:58, 23 August 2010

Good idea, thanks! I checked in this change to the code.

Status & tagging log