Skip to content

Commit 6bc3f7c

Browse files
DigiLiveDigiLive
committed
Code Optimization, cleanup, refactoring and commenting.
Fixes: Method HtmlArray::fixSpaces() replaced leading space characters with the wrong amount of ' ' characters instead of the amount of spaces as defined in HtmlArray::options. This method is removed and replacement of these spaces has moved to method HtmlArray::formatLines() wich replaces the leading spaces with the correct amount of non-breaking spaces (character ' ').
1 parent 5e7ddf3 commit 6bc3f7c

File tree

2 files changed

+136
-121
lines changed

2 files changed

+136
-121
lines changed

‎lib/jblond/Diff/Renderer/Html/HtmlArray.php

Lines changed: 135 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -21,117 +21,134 @@
2121
class HtmlArray extends RendererAbstract
2222
{
2323
/**
24-
* @var array Array of the default options that apply to this renderer.
24+
* @var array Associative array containing the default options available for this renderer and their default
25+
* value.
26+
* - tabSize The amount of spaces to replace a tab character with.
27+
* - title_a Title of the "old" version of text.
28+
* - title_b Title of the "new" version of text.
2529
*/
26-
protected $defaultOptions = array(
30+
protected $defaultOptions = [
2731
'tabSize' => 4,
2832
'title_a' => 'Old Version',
2933
'title_b' => 'New Version',
30-
);
34+
];
3135

3236
/**
33-
* @param string|array $changes
34-
* @param SideBySide|Inline $object
35-
* @return string
37+
* Generate a string representation of changes between the "old and "new" sequences.
38+
*
39+
* This method is called by the renderers which extends this class.
40+
*
41+
* @param array $changes Contains the op-codes about the differences between "old and "new".
42+
* @param SideBySide|Inline $htmlRenderer Renderer which extends this class.
43+
*
44+
* @return string HTML representation of the differences.
3645
*/
37-
public function renderHtml($changes, $object)
46+
public function renderHtml(array $changes, object $htmlRenderer): string
3847
{
39-
$html = '';
4048
if (empty($changes)) {
41-
return $html;
49+
//No changes between "old" and "new"
50+
return 'No differences found.';
4251
}
4352

44-
$html .= $object->generateTableHeader();
53+
$html = $htmlRenderer->generateTableHeader();
4554

4655
foreach ($changes as $i => $blocks) {
47-
// If this is a separate block, we're condensing code so output ...,
48-
// indicating a significant portion of the code has been collapsed as
49-
// it is the same
56+
if ($i > 0) {
57+
// If this is a separate block, we're condensing code so output ...,
58+
// indicating a significant portion of the code has been collapsed as
59+
// it is the same.
60+
//TODO: When is $i > 0 ?
61+
$html .= '<span class="Skipped"><br>&hellip;<br></span>';
62+
}
5063

5164
foreach ($blocks as $change) {
5265
$html .= '<tbody class="Change' . ucfirst($change['tag']) . '">';
5366
switch ($change['tag']) {
5467
// Equal changes should be shown on both sides of the diff
5568
case 'equal':
56-
$html .= $object->generateTableRowsEqual($change);
69+
$html .= $htmlRenderer->generateTableRowsEqual($change);
5770
break;
5871
// Added lines only on the right side
5972
case 'insert':
60-
$html .= $object->generateTableRowsInsert($change);
73+
$html .= $htmlRenderer->generateTableRowsInsert($change);
6174
break;
6275
// Show deleted lines only on the left side
6376
case 'delete':
64-
$html .= $object->generateTableRowsDelete($change);
77+
$html .= $htmlRenderer->generateTableRowsDelete($change);
6578
break;
6679
// Show modified lines on both sides
6780
case 'replace':
68-
$html .= $object->generateTableRowsReplace($change);
81+
$html .= $htmlRenderer->generateTableRowsReplace($change);
6982
break;
7083
}
84+
7185
$html .= '</tbody>';
7286
}
7387
}
88+
7489
$html .= '</table>';
90+
7591
return $html;
7692
}
93+
7794
/**
78-
* Render and return an array structure suitable for generating HTML
79-
* based differences. Generally called by subclasses that generate a
80-
* HTML based diff and return an array of the changes to show in the diff.
95+
* Render and return an array structure suitable for generating HTML based differences.
8196
*
82-
* @return array|string An array of the generated changes, suitable for presentation in HTML.
97+
* Generally called by classes which extend this class and that generate a HTML based diff by returning an array of
98+
* the changes to show in the diff.
99+
*
100+
* @return array An array of the generated changes, suitable for presentation in HTML.
83101
*/
84102
public function render()
85103
{
86-
// As we'll be modifying old & new to include our change markers,
87-
// we need to get the contents and store them here. That way
88-
// we're not going to destroy the original data
104+
// "old" & "new" are copied so change markers can be added without modifying the original sequences.
89105
$old = $this->diff->getOld();
90106
$new = $this->diff->getNew();
91107

92-
$changes = array();
108+
$changes = [];
93109
$opCodes = $this->diff->getGroupedOpcodes();
110+
94111
foreach ($opCodes as $group) {
95-
$blocks = array();
96-
$lastTag = null;
97-
$lastBlock = 0;
112+
$blocks = [];
113+
$lastTag = null;
114+
$lastBlock = 0;
98115
foreach ($group as $code) {
99116
list($tag, $i1, $i2, $j1, $j2) = $code;
100117

101118
if ($tag == 'replace' && $i2 - $i1 == $j2 - $j1) {
102119
for ($i = 0; $i < ($i2 - $i1); ++$i) {
103-
$fromLine = $old[$i1 + $i];
104-
$toLine = $new[$j1 + $i];
120+
$fromLine = $old[$i1 + $i];
121+
$toLine = $new[$j1 + $i];
105122

106123
list($start, $end) = $this->getChangeExtent($fromLine, $toLine);
107124
if ($start != 0 || $end != 0) {
108-
$realEnd = mb_strlen($fromLine) + $end;
125+
$realEnd = mb_strlen($fromLine) + $end;
126+
$fromLine = mb_substr($fromLine, 0, $start) . "\0" .
127+
mb_substr($fromLine, $start, $realEnd - $start) . "\1" .
128+
mb_substr($fromLine, $realEnd);
109129

110-
$fromLine = mb_substr($fromLine, 0, $start) . "\0" .
111-
mb_substr($fromLine, $start, $realEnd - $start) . "\1" . mb_substr($fromLine, $realEnd);
130+
$realEnd = mb_strlen($toLine) + $end;
131+
$toLine = mb_substr($toLine, 0, $start) . "\0" .
132+
mb_substr($toLine, $start, $realEnd - $start) . "\1" .
133+
mb_substr($toLine, $realEnd);
112134

113-
$realEnd = mb_strlen($toLine) + $end;
114-
115-
$toLine = mb_substr($toLine, 0, $start) .
116-
"\0" . mb_substr($toLine, $start, $realEnd - $start) . "\1" .
117-
mb_substr($toLine, $realEnd);
118-
119-
$old[$i1 + $i] = $fromLine;
120-
$new[$j1 + $i] = $toLine;
135+
$old[$i1 + $i] = $fromLine;
136+
$new[$j1 + $i] = $toLine;
121137
}
122138
}
123139
}
124140

125141
if ($tag != $lastTag) {
126-
$blocks[] = $this->getDefaultArray($tag, $i1, $j1);
127-
$lastBlock = count($blocks) - 1;
142+
$blocks[] = $this->getDefaultArray($tag, $i1, $j1);
143+
$lastBlock = count($blocks) - 1;
128144
}
129145

130146
$lastTag = $tag;
131147

132148
if ($tag == 'equal') {
133149
$lines = array_slice($old, $i1, ($i2 - $i1));
134150
$blocks[$lastBlock]['base']['lines'] += $this->formatLines($lines);
151+
135152
$lines = array_slice($new, $j1, ($j2 - $j1));
136153
$blocks[$lastBlock]['changed']['lines'] += $this->formatLines($lines);
137154
} else {
@@ -152,113 +169,109 @@ public function render()
152169
}
153170
$changes[] = $blocks;
154171
}
172+
155173
return $changes;
156174
}
157175

158176
/**
159-
* Given two strings, determine where the changes in the two strings
160-
* begin, and where the changes in the two strings end.
177+
* Determine where changes in two strings begin and where they end.
178+
*
179+
* This returns an array.
180+
* The first value defines the first (starting at 0) character from start of the old string which is different.
181+
* The second value defines the last character from end of the old string which is different.
182+
*
183+
*
184+
* @param string $oldString The first string to compare.
185+
* @param string $newString The second string to compare.
161186
*
162-
* @param string $fromLine The first string.
163-
* @param string $toLine The second string.
164187
* @return array Array containing the starting position (0 by default) and the ending position (-1 by default)
165188
*/
166-
private function getChangeExtent(string $fromLine, string $toLine)
189+
private function getChangeExtent(string $oldString, string $newString): array
167190
{
168191
$start = 0;
169-
$limit = min(mb_strlen($fromLine), mb_strlen($toLine));
170-
while ($start < $limit && mb_substr($fromLine, $start, 1) == mb_substr($toLine, $start, 1)) {
192+
$limit = min(mb_strlen($oldString), mb_strlen($newString));
193+
194+
// Find first difference.
195+
while ($start < $limit && mb_substr($oldString, $start, 1) == mb_substr($newString, $start, 1)) {
171196
++$start;
172197
}
173-
$end = -1;
174-
$limit = $limit - $start;
175-
while (-$end <= $limit && mb_substr($fromLine, $end, 1) == mb_substr($toLine, $end, 1)) {
198+
199+
$end = -1;
200+
$limit = $limit - $start;
201+
202+
// Find last difference.
203+
while (-$end <= $limit && mb_substr($oldString, $end, 1) == mb_substr($newString, $end, 1)) {
176204
--$end;
177205
}
178-
return array(
206+
207+
return [
179208
$start,
180209
$end + 1
181-
);
210+
];
182211
}
183212

184213
/**
185-
* Format a series of lines suitable for output in a HTML rendered diff.
186-
* This involves replacing tab characters with spaces, making the HTML safe
187-
* for output, ensuring that double spaces are replaced with &#xA0; etc.
214+
* Format a series of strings which are suitable for output in a HTML rendered diff.
215+
*
216+
* This involves replacing tab characters with spaces, making the HTML safe for output by ensuring that double
217+
* spaces are replaced with &nbsp; etc.
188218
*
189-
* @param array $lines Array of lines to format.
190-
* @return array Array of the formatted lines.
219+
* @param array $lines Array of strings to format.
220+
*
221+
* @return array Array of formatted strings.
191222
*/
192-
protected function formatLines(array $lines): array
223+
protected function formatLines(array $strings): array
193224
{
194225
if ($this->options['tabSize'] !== false) {
195-
$lines = array_map(
226+
// Replace tabs with spaces.
227+
$strings = array_map(
196228
function ($item) {
197229
return $this->expandTabs($item);
198230
},
199-
$lines
231+
$strings
200232
);
201233
}
202-
$lines = array_map(
234+
235+
// Convert special characters to HTML entities
236+
$strings = array_map(
203237
function ($item) {
204238
return $this->htmlSafe($item);
205239
},
206-
$lines
240+
$strings
207241
);
208-
foreach ($lines as &$line) {
209-
$line = preg_replace_callback('# ( +)|^ #', array($this, 'fixSpaces'), $line);
210-
}
211-
return $lines;
212-
}
213242

214-
/**
215-
* Replace a string containing spaces with a HTML representation using &#xA0;.
216-
*
217-
* @param array $matches The string of spaces.
218-
* @return string The HTML representation of the string.
219-
*/
220-
protected function fixSpaces(array $matches): string
221-
{
222-
$buffer = '';
223-
$count = 0;
224-
foreach ($matches as $spaces) {
225-
$count = strlen($spaces);
226-
if ($count == 0) {
227-
continue;
228-
}
229-
$div = (int) ($count / 2);
230-
$mod = $count % 2;
231-
$buffer .= str_repeat('&#xA0; ', $div) . str_repeat('&#xA0;', $mod);
243+
// Replace leading spaces of a line with HTML enities.
244+
foreach ($strings as &$line) {
245+
$line = preg_replace_callback(
246+
'/(^[ \0\1]*)/',
247+
function($matches) {
248+
return str_replace(' ', "&nbsp;", $matches[0]);
249+
},
250+
$line
251+
);
232252
}
253+
unset($line);
233254

234-
$div = (int) ($count / 2);
235-
$mod = $count % 2;
236-
return str_repeat('&#xA0; ', $div) . str_repeat('&#xA0;', $mod);
255+
return $strings;
237256
}
238257

239258
/**
240-
* Replace tabs in a single line with a number of spaces as defined by the tabSize option.
259+
* Replace tabs in a string with an amount of spaces as defined by the tabSize option of this class.
260+
*
261+
* @param string $line The string which contains tabs to convert.
241262
*
242-
* @param string $line The containing tabs to convert.
243263
* @return string The line with the tabs converted to spaces.
244264
*/
245265
private function expandTabs(string $line): string
246266
{
247-
$tabSize = $this->options['tabSize'];
248-
while (($pos = strpos($line, "\t")) !== false) {
249-
$left = substr($line, 0, $pos);
250-
$right = substr($line, $pos + 1);
251-
$length = $tabSize - ($pos % $tabSize);
252-
$spaces = str_repeat(' ', $length);
253-
$line = $left . $spaces . $right;
254-
}
255-
return $line;
267+
return str_replace("\t", str_repeat(' ', $this->options['tabSize']), $line);
256268
}
257269

258270
/**
259-
* Make a string containing HTML safe for output on a page.
271+
* Make a string HTML safe for output on a page.
272+
*
273+
* @param string $string The string to make safe.
260274
*
261-
* @param string $string The string.
262275
* @return string The string with the HTML characters replaced by entities.
263276
*/
264277
private function htmlSafe(string $string): string
@@ -267,24 +280,26 @@ private function htmlSafe(string $string): string
267280
}
268281

269282
/**
270-
* @param string $tag
271-
* @param integer $i1
272-
* @param integer $j1
283+
* Helper function that provides an array for the renderer with default values for the changes to render.
284+
*
285+
* @param string $tag Kind of difference.
286+
* @param integer $lineInOld Start of block in "old".
287+
* @param integer $lineInNew Start of block in "new".
288+
*
273289
* @return array
274290
*/
275-
private function getDefaultArray(string $tag, int $i1, int $j1): array
291+
private function getDefaultArray(string $tag, int $lineInOld, int $lineInNew): array
276292
{
277-
return array
278-
(
279-
'tag' => $tag,
280-
'base' => array(
281-
'offset' => $i1,
282-
'lines' => array()
283-
),
284-
'changed' => array(
285-
'offset' => $j1,
286-
'lines' => array()
287-
)
288-
);
293+
return [
294+
'tag' => $tag,
295+
'base' => [
296+
'offset' => $lineInOld,
297+
'lines' => []
298+
],
299+
'changed' => [
300+
'offset' => $lineInNew,
301+
'lines' => []
302+
]
303+
];
289304
}
290305
}

0 commit comments

Comments
 (0)