mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-23 22:10:55 +01:00
Pick context windows for inlines in a slightly smarter way
Summary: Ref T10694. This mostly prevents us from having a degenerate case if someone leaves a 200-line inline. - For one-line inlines, show 1 line of context above and below (3 lines total). - For 3+ line inlines, show just the inline. - For 7+ line inlines, show only the first part. Test Plan: Made a bunch of weird long/short/different-sized comments, saw reasonble-appearing context in text and HTML mail output. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10694 Differential Revision: https://secure.phabricator.com/D15853
This commit is contained in:
parent
94c7bb605c
commit
1baef494c1
3 changed files with 26 additions and 3 deletions
|
@ -290,9 +290,27 @@ final class DifferentialInlineCommentMailView
|
||||||
$start = $comment->getLineNumber();
|
$start = $comment->getLineNumber();
|
||||||
$length = $comment->getLineLength();
|
$length = $comment->getLineLength();
|
||||||
|
|
||||||
|
// By default, show one line of context around the target inline.
|
||||||
|
$context = 1;
|
||||||
|
|
||||||
|
// If the inline is at least 3 lines long, don't show any extra context.
|
||||||
|
if ($length >= 2) {
|
||||||
|
$context = 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
// If the inline is more than 7 lines long, only show the first 7 lines.
|
||||||
|
if ($length >= 6) {
|
||||||
|
$length = 6;
|
||||||
|
}
|
||||||
|
|
||||||
if (!$is_html) {
|
if (!$is_html) {
|
||||||
$hunks = $changeset->getHunks();
|
$hunks = $changeset->getHunks();
|
||||||
$patch = $parser->makeContextDiff($hunks, $is_new, $start, $length, 1);
|
$patch = $parser->makeContextDiff(
|
||||||
|
$hunks,
|
||||||
|
$is_new,
|
||||||
|
$start,
|
||||||
|
$length,
|
||||||
|
$context);
|
||||||
$patch = phutil_split_lines($patch);
|
$patch = phutil_split_lines($patch);
|
||||||
|
|
||||||
// Remove the "@@ -x,y +u,v @@" line.
|
// Remove the "@@ -x,y +u,v @@" line.
|
||||||
|
@ -318,7 +336,10 @@ final class DifferentialInlineCommentMailView
|
||||||
|
|
||||||
$parser->setRenderer(new DifferentialChangesetOneUpMailRenderer());
|
$parser->setRenderer(new DifferentialChangesetOneUpMailRenderer());
|
||||||
|
|
||||||
return $parser->render($start - 1, $length + 3, array());
|
return $parser->render(
|
||||||
|
$start - $context,
|
||||||
|
$length + 1 + (2 * $context),
|
||||||
|
array());
|
||||||
}
|
}
|
||||||
|
|
||||||
private function renderPatch(
|
private function renderPatch(
|
||||||
|
|
|
@ -852,7 +852,7 @@ final class DifferentialChangesetParser extends Phobject {
|
||||||
|
|
||||||
$range_end = $this->getOffset($offset_map, $range_start + $range_len);
|
$range_end = $this->getOffset($offset_map, $range_start + $range_len);
|
||||||
$range_start = $this->getOffset($offset_map, $range_start);
|
$range_start = $this->getOffset($offset_map, $range_start);
|
||||||
$range_len = $range_end - $range_start;
|
$range_len = ($range_end - $range_start);
|
||||||
}
|
}
|
||||||
|
|
||||||
$render_pch = $this->shouldRenderPropertyChangeHeader($this->changeset);
|
$render_pch = $this->shouldRenderPropertyChangeHeader($this->changeset);
|
||||||
|
|
|
@ -73,6 +73,8 @@ final class DifferentialChangesetOneUpMailRenderer
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$style = "padding: 0 2px; {$style}";
|
||||||
|
|
||||||
$out[] = phutil_tag(
|
$out[] = phutil_tag(
|
||||||
'div',
|
'div',
|
||||||
array(
|
array(
|
||||||
|
|
Loading…
Reference in a new issue