Skip to content

Commit

Permalink
Refactor code to make it simpler.
Browse files Browse the repository at this point in the history
The changes should also:
- fix unlikely edge case when replacement line is the same as the
  old line (would have resulted in timeout)
- reduce memory footprint
- avoid applying string search beyond maxlines replacement limit
  • Loading branch information
Chris--S committed May 28, 2015
1 parent 2b71c2e commit 9a734b7
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 22 deletions.
28 changes: 26 additions & 2 deletions _test/tests/inc/io_replaceinfile.test.php
Expand Up @@ -2,6 +2,8 @@

class io_replaceinfile_test extends DokuWikiTest {

protected $contents = "The\012Delete\012Delete\012Delete01\012Delete02\012Delete\012DeleteX\012Test\012";

/*
* dependency for tests needing zlib extension to pass
*/
Expand All @@ -21,8 +23,8 @@ public function test_ext_bz2() {
}

function _write($file){
$contents = "The\012Delete\012Delete\012Delete01\012Delete02\012Delete\012DeleteX\012Test\012";
io_saveFile($file, $contents);

io_saveFile($file, $this->contents);
// Replace one, no regex
$this->assertTrue(io_replaceInFile($file, "Delete\012", "Delete00\012", false, 1));
$this->assertEquals("The\012Delete00\012Delete\012Delete01\012Delete02\012Delete\012DeleteX\012Test\012", io_readFile($file));
Expand All @@ -41,6 +43,7 @@ function test_replace(){
$this->_write(TMP_DIR.'/test.txt');
}


/**
* @depends test_ext_zlib
*/
Expand All @@ -55,4 +58,25 @@ function test_bzwrite(){
$this->_write(TMP_DIR.'/test.txt.bz2');
}

/**
*
*/
function test_edgecase1()
{
$file = TMP_DIR . '/test.txt';
// Replace all, no regex, backreference like construct in replacement line
io_saveFile($file, $this->contents);
$this->assertTrue(io_replaceInFile($file, "Delete\012", "Delete\\00\012", false, -1));
$this->assertEquals("The\012Delete\\00\012Delete\\00\012Delete01\012Delete02\012Delete\\00\012DeleteX\012Test\012", io_readFile($file), "Edge case: backreference like construct in replacement line");
}
/**
* @small
*/
function test_edgecase2() {
$file = TMP_DIR.'/test.txt';
// Replace all, no regex, replacement line == search line
io_saveFile($file, $this->contents);
$this->assertTrue(io_replaceInFile($file, "Delete\012", "Delete\012", false, -1));
$this->assertEquals("The\012Delete\012Delete\012Delete01\012Delete02\012Delete\012DeleteX\012Test\012", io_readFile($file), "Edge case: new line the same as old line");
}
}
35 changes: 15 additions & 20 deletions inc/io.php
Expand Up @@ -311,28 +311,23 @@ function io_replaceInFile($file, $oldline, $newline, $regex=false, $maxlines=0)
$lines = file($file);
}

// remove all matching lines
if ($regex) {
if($maxlines > 0) {
$matches = preg_grep($oldline, $lines);
$count = 0;
foreach($matches as $ix=>$m) {
$lines[$ix] = preg_replace($oldline, $newline, $m);
if(++$count >= $maxlines) break;
}
} else {
$lines = ($maxlines == 0) ? preg_grep($oldline, $lines, PREG_GREP_INVERT)
: preg_replace($oldline, $newline, $lines, $maxlines);
}
} else {
// make non-regexes into regexes
$pattern = $regex ? $oldline : '/'.preg_quote($oldline,'/').'/';
$replace = $regex ? $newline : addcslashes($newline, '\$');

// remove matching lines
if ($maxlines > 0) {
$count = 0;
$replaceline = $maxlines == 0 ? '' : $newline;
$pos = array_search($oldline,$lines); //return null or false if not found
while(is_int($pos)){
$lines[$pos] = $replaceline;
if($maxlines > 0 && ++$count >= $maxlines) break;
$pos = array_search($oldline,$lines);
$matched = 0;
while (($count < $maxlines) && (list($i,$line) = each($lines))) {
// $matched will be set to 0|1 depending on whether pattern is matched and line replaced
$lines[$i] = preg_replace($pattern, $replace, $line, -1, $matched);
if ($matched) $count++;

This comment has been minimized.

Copy link
@Chris--S

Chris--S May 28, 2015

Author Collaborator

actual code chosen as more understandable than

$count += $matched; 
}
} else {
$lines = ($maxlines == 0) ?
preg_grep($pattern, $lines, PREG_GREP_INVERT) :
preg_replace($pattern, $replace, $lines);
}

if($maxlines == 0 && ((string)$newline) !== '') {
Expand Down

0 comments on commit 9a734b7

Please sign in to comment.