From 20c3f14cc88d2245b9ba04ab6dd858e7a2f66f81 Mon Sep 17 00:00:00 2001 From: Thomas Keller Date: Thu, 2 Jun 2011 00:48:38 +0200 Subject: [PATCH] git and hg diff views did not show empty context lines, a regression from the commit(s) from issue 633. The diff parser assumed a properly formatted diff that denotes empty context lines with a single space in the first column. This single space however was missing, because the hg and git backends got the diff through PHP's exec() function and this returns already line-splitted output, but - and this is the actual problem - removes trailing whitespace at the end of each line, essentially making " \n" only "\n". When splitting this string now again with PREG_SPLIT_NO_EMPTY the empty line was completely lost in the diff output. To make it clear that an empty line does not mark a context line now, but should stop the diff parsing, the Diff parser now also defaults to 'false' as line type. This commit fixes issue 688. --- NEWS.mdtext | 1 + src/IDF/Diff.php | 2 +- src/IDF/Scm.php | 23 +++++++++++++++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/NEWS.mdtext b/NEWS.mdtext index 87bb58c..71f04bf 100644 --- a/NEWS.mdtext +++ b/NEWS.mdtext @@ -14,6 +14,7 @@ - Timeline only displays filter options for items a user has actually access to (issue 655) - The log, tags and branches parsers for Mercurial are more robust now (issue 663) - Fix SSH public key parsing issues and improve the check for existing, uploaded keys (issue 679) +- Diff views now show empty context lines for git and hg again (issue 688) - Let the SVN command line client not store the login credentials we give him as arguments ## Documentation diff --git a/src/IDF/Diff.php b/src/IDF/Diff.php index 4bd3abc..8909618 100644 --- a/src/IDF/Diff.php +++ b/src/IDF/Diff.php @@ -101,7 +101,7 @@ class IDF_Diff $files[$current_file]['chunks'][] = array(); while ($i < $diffsize && ($addlines >= 0 || $dellines >= 0)) { - $linetype = $this->lines[$i] != '' ? $this->lines[$i][0] : ' '; + $linetype = $this->lines[$i] != '' ? $this->lines[$i][0] : false; switch ($linetype) { case ' ': $files[$current_file]['chunks'][$current_chunk][] = diff --git a/src/IDF/Scm.php b/src/IDF/Scm.php index 6c0d866..e34c989 100644 --- a/src/IDF/Scm.php +++ b/src/IDF/Scm.php @@ -88,22 +88,37 @@ class IDF_Scm } /** - * Run exec and log some information. + * Runs the given command and log some information. + * + * A previous version used plain exec(), but this should not be used + * for various reasons, one being that this command does not preserve + * trailing whitespace, which is essential for proper diff parsing. * * @param $caller Calling method * @param $cmd Command to run * @param &$out Array of output * @param &$return Return value - * @return string Last line of the command */ public static function exec($caller, $cmd, &$out=null, &$return=null) { + $return = 1; Pluf_Log::stime('timer'); - $ret = exec($cmd, $out, $return); + $fp = popen($cmd, 'r'); + $buf = ''; + if ($fp !== false) { + $return = 0; + while (!feof($fp)) { + $buf .= fread($fp, 1024); + } + pclose($fp); + } + $out = preg_split('/\r\n|\r|\n/', $buf); + $elem = count($out); + if ($elem > 0 && $out[$elem-1] === '') + unset($out[$elem-1]); Pluf_Log::perf(array($caller, $cmd, Pluf_Log::etime('timer', 'total_exec'))); Pluf_Log::debug(array($caller, $cmd, $out)); Pluf_Log::inc('exec_calls'); - return $ret; } /**