Multi-line highlighting in Diffusion.

Summary: Currently, Diffusion supports highlighting of line ranges passed in the URI. It would be helpful to be able to highlight multiple line ranges.

Test Plan: Accessed directly via URL in my sandbox. Seems to work. I'm not sure what other components use this functionality, but this change should be backwards compatible.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D2921
This commit is contained in:
Miles Shang
2012-07-04 14:04:45 -07:00
parent 731e0df2b5
commit 1b8ed98ddc
3 changed files with 37 additions and 14 deletions

View File

@@ -293,15 +293,22 @@ final class DiffusionBrowseFileController extends DiffusionController {
$epoch_range = ($epoch_max - $epoch_min) + 1; $epoch_range = ($epoch_max - $epoch_min) + 1;
} }
$min_line = 0; $line_arr = array();
$line = $drequest->getLine(); $line_str = $drequest->getLine();
if (strpos($line, '-') !== false) { $ranges = explode(',', $line_str);
list($min, $max) = explode('-', $line, 2); foreach ($ranges as $range) {
$min_line = min($min, $max); if (strpos($range, '-') !== false) {
$max_line = max($min, $max); list($min, $max) = explode('-', $range, 2);
} else if (strlen($line)) { $line_arr[] = array(
$min_line = $line; 'min' => min($min, $max),
$max_line = $line; 'max' => max($min, $max),
);
} else if (strlen($range)) {
$line_arr[] = array(
'min' => $range,
'max' => $range,
);
}
} }
$display = array(); $display = array();
@@ -366,12 +373,15 @@ final class DiffusionBrowseFileController extends DiffusionController {
} }
} }
if ($min_line) { if ($line_arr) {
if ($line_number == $min_line) { if ($line_number == $line_arr[0]['min']) {
$display_line['target'] = true; $display_line['target'] = true;
} }
if ($line_number >= $min_line && $line_number <= $max_line) { foreach ($line_arr as $range) {
$display_line['highlighted'] = true; if ($line_number >= $range['min'] &&
$line_number <= $range['max']) {
$display_line['highlighted'] = true;
}
} }
} }

View File

@@ -475,7 +475,7 @@ abstract class DiffusionRequest {
// Consume the back part of the URI, up to the first "$". Use a negative // Consume the back part of the URI, up to the first "$". Use a negative
// lookbehind to prevent matching '$$'. We double the '$' symbol when // lookbehind to prevent matching '$$'. We double the '$' symbol when
// encoding so that files with names like "money/$100" will survive. // encoding so that files with names like "money/$100" will survive.
$pattern = '@(?:(?:^|[^$])(?:[$][$])*)[$]([\d-]+)$@'; $pattern = '@(?:(?:^|[^$])(?:[$][$])*)[$]([\d-,]+)$@';
if (preg_match($pattern, $blob, $matches)) { if (preg_match($pattern, $blob, $matches)) {
$result['line'] = $matches[1]; $result['line'] = $matches[1];
$blob = substr($blob, 0, -(strlen($matches[1]) + 1)); $blob = substr($blob, 0, -(strlen($matches[1]) + 1));

View File

@@ -55,6 +55,12 @@ final class DiffusionURITestCase extends ArcanistPhutilTestCase {
'commit' => '$;;semicolon;;$$', 'commit' => '$;;semicolon;;$$',
'line' => '100', 'line' => '100',
), ),
'branch/path.ext;abc$3-5,7-12,14' => array(
'branch' => 'branch',
'path' => 'path.ext',
'commit' => 'abc',
'line' => '3-5,7-12,14',
),
); );
foreach ($map as $input => $expect) { foreach ($map as $input => $expect) {
@@ -140,6 +146,13 @@ final class DiffusionURITestCase extends ArcanistPhutilTestCase {
'path' => 'path/to/file.ext', 'path' => 'path/to/file.ext',
'commit' => 'abc', 'commit' => 'abc',
), ),
'/diffusion/A/browse/branch/path.ext$3-5%2C7-12%2C14' => array(
'action' => 'browse',
'callsign' => 'A',
'branch' => 'branch',
'path' => 'path.ext',
'line' => '3-5,7-12,14',
),
); );
foreach ($map as $expect => $input) { foreach ($map as $expect => $input) {