MediaWiki r110923 - Code Review

Repository:	MediaWiki
Revision:	< r110922‎ \| r110923 (on ViewVC)‎ \| r110924 >
Date:	09:16, 8 February 2012
Author:	hashar
Status:	resolved (Comments)
Tags:
Comment:	use rtrim() to strip trailing slashes rtrim() can be given a list of character to strip from a given string.
Modified paths:	/trunk/phase3/includes/filerepo/backend/FSFileBackend.php (modified) (history) (diff)

Diff [purge]

Index: trunk/phase3/includes/filerepo/backend/FSFileBackend.php
—	—	@@ -39,18 +39,18 @@
40	40	*/
41	41	public function __construct( array $config ) {
42	42	parent::__construct( $config );
	43	+
	44	+ // Remove any possible trailing slash from directories
	45	+
43	46	if ( isset( $config['basePath'] ) ) {
44		~~- if ( substr( $this->basePath, -1 ) === '/' ) {~~
45		~~- $this->basePath = substr( $this->basePath, 0, -1 ); // remove trailing slash~~
46		~~- }~~
	47	+ rtrim( $this->basePath, '/' ); // remove trailing slash
47	48	} else {
48	49	$this->basePath = null; // none; containers must have explicit paths
49	50	}
	51	+
50	52	$this->containerPaths = (array)$config['containerPaths'];
51	53	foreach ( $this->containerPaths as &$path ) {
52		~~- if ( substr( $path, -1 ) === '/' ) {~~
53		~~- $path = substr( $path, 0, -1 ); // remove trailing slash~~
54		~~- }~~
	54	+ rtrim( $path, '/' ); // remove trailing slash
55	55	}
56	56	$this->fileMode = isset( $config['fileMode'] )
57	57	? $config['fileMode']

Rev.	Commit summary	Author	Date
r110926	r110923 made basePath always null!	hashar	09:54, 8 February 2012
r110934	rtrim() call does not pass by reference!...	hashar	14:30, 8 February 2012

#Comment by Hashar (Talk | contribs) 09:55, 8 February 2012

That revision made basePath always null. Fixed with r110926

#Comment by Aaron Schulz (Talk | contribs) 10:42, 8 February 2012

The change in the foreach loop is broken since rtrim does not work by reference.

16:34, 8 February 2012 Aaron Schulz (Talk | contribs) changed the status of r110923 [removed: new added: resolved]
14:30, 8 February 2012 Hashar (Talk | contribs) changed the status of r110923 [removed: fixme added: new]
10:42, 8 February 2012 Aaron Schulz (Talk | contribs) changed the status of r110923 [removed: new added: fixme]