r68034 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r68033‎ | r68034 | r68035 >
Date:21:39, 14 June 2010
Author:platonides
Status:ok
Tags:
Comment:
Fix grep regex, move out the inner loop, aggregate all second-stage sed calls into one and remove usage of non-portable sed -i.

Mac OS and FreeBSD sed takes "sed -i -e <expression>" as meaning "use -e as backup suffix", they need sed -i '' -e <expression>.
We could please both FreeBSD sed and GNU sed with sed -i~ -e <expression> and then deleting the ~ file, but that's ugly.
Moreover, OpenBSD sed doesn't accept any kind of -i option.

So directly remove -i usage, and use a temporary file with .counter suffix. ~ would be tipical, but since that's also usd by some text editors, using .counter we shoulfn't clash with anyone. Note that getting a .counter file is a bug which should be reported.
Modified paths:
  • /trunk/extensions/UsabilityInitiative/Makefile (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/Makefile
——@@ -149,10 +149,11 @@
150150 # Use sed to replace the line for that file with '+1' appended to the version.
151151 # Note that $${basefile//\//\\/} expands in the shell to $basefile with all slashes escaped.
152152 # End for
153 -# For each group of summing numbers inside the target file, use bc to calculate the sum and replace with sed.
 153+# For each group of summing numbers inside the target file, use shell arithmetic expansion to calculate the sum.
 154+# Replace all numbers at once with sed. The replacements are preceded have a leading ; an empty expression about which sed doesn't care.
154155 %.hooks.php: $(WIKIEDITOR_HOOKS)
155 - for file in $?; do basefile="$${file#$(shell echo $* | sed "s/\([^\/]*\/\).*/\\1/")}"; sed -i -e "s/\(.*'src' => '$${basefile//\//\\/}', 'version' => \)\([0-9+]*\)\(.*\)/\\1\\2+1\\3/" $@; \
156 - for i in $$(grep --only-matching -P " ([0-9]+(\+[0-9]))+ " $@); do sed -i -e "s/ $$i / $$(($$i)) /" $@; done; done
 156+ for file in $?; do basefile="$${file#$(shell echo $* | sed "s/\([^\/]*\/\).*/\\1/")}"; sed -e "s/\(.*'src' => '$${basefile//\//\\/}', 'version' => \)\([0-9+]*\)\(.*\)/\\1\\2+1\\3/" $@ > $@.counter; done; \
 157+ RE=""; for i in $$(grep --only-matching -P " ([0-9]+(\+[0-9])+) " $@.counter); do RE="$$RE;s/ $$i / $$(($$i)) /"; done; sed -e "$$RE" $@.counter > $@ && rm $@.counter
157158
158159 # Actions
159160

Follow-up revisions

RevisionCommit summaryAuthorDate
r68036Fix bug introduced in r68034 where only the minified file get bumped....platonides22:24, 14 June 2010

Status & tagging log