revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-22 19:49:02 +01:00
Code Issues Releases Wiki Activity

Unify changeset line ID rendering and bring it to unified diffs

Browse source 
Summary:
Ref T2009. Currently, lines don't get their "C123NL456" IDs set in the unified view. This is the major way that inlines are glued to changesets.

Simplify this rendering and bring it into the HTML renderer, then use it in the OneUp renderer.

Test Plan:
  - Interacted with side-by-side inlines (hovered, added, edited, deleted), saw unchanged behavior.
  - Interacted with unified inlines. They still don't work, but the error that breaks them is deeper in the stack.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D11983
This commit is contained in:
epriestley 2015-03-05 14:03:57 -08:00
parent cb886eb60e
commit 35c1dbf1f8
3 changed files with 73 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
src/applications/differential/render/DifferentialChangesetHTMLRenderer.php
View file

@ -525,5 +525,48 @@ abstract class DifferentialChangesetHTMLRenderer
$text);
}
/**
* Build the prefixes for line IDs used to track inline comments.
*
* @return pair<wild, wild> Left and right prefixes.
*/
protected function getLineIDPrefixes() {
// These look like "C123NL45", which means the line is line 45 on the
// "new" side of the file in changeset 123.
// The "C" stands for "changeset", and is followed by a changeset ID.
// "N" stands for "new" and means the comment should attach to the new file
// when stored. "O" stands for "old" and means the comment should attach to
// the old file. These are important because either the old or new part
// of a file may appear on the left or right side of the diff in the
// diff-of-diffs view.
// The "L" stands for "line" and is followed by the line number.
if ($this->getOldChangesetID()) {
$left_prefix = array();
$left_prefix[] = 'C';
$left_prefix[] = $this->getOldChangesetID();
$left_prefix[] = $this->getOldAttachesToNewFile() ? 'N' : 'O';
$left_prefix[] = 'L';
$left_prefix = implode('', $left_prefix);
} else {
$left_prefix = null;
}
if ($this->getNewChangesetID()) {
$right_prefix = array();
$right_prefix[] = 'C';
$right_prefix[] = $this->getNewChangesetID();
$right_prefix[] = $this->getNewAttachesToNewFile() ? 'N' : 'O';
$right_prefix[] = 'L';
$right_prefix = implode('', $right_prefix);
} else {
$right_prefix = null;
}
return array($left_prefix, $right_prefix);
}
}

29
src/applications/differential/render/DifferentialChangesetOneUpRenderer.php
View file

@ -30,6 +30,8 @@ final class DifferentialChangesetOneUpRenderer
$primitives = $this->buildPrimitives($range_start, $range_len);
list($left_prefix, $right_prefix) = $this->getLineIDPrefixes();
$out = array();
foreach ($primitives as $p) {
$type = $p['type'];
@ -43,18 +45,37 @@ final class DifferentialChangesetOneUpRenderer
} else {
$class = 'left';
}
$out[] = phutil_tag('th', array(), $p['line']);
if ($left_prefix) {
$left_id = $left_prefix.$p['line'];
} else {
$left_id = null;
}
$out[] = phutil_tag('th', array('id' => $left_id), $p['line']);
$out[] = phutil_tag('th', array());
$out[] = phutil_tag('td', array('class' => $class), $p['render']);
} else if ($type == 'new') {
} else {
if ($p['htype']) {
$class = 'right new';
$out[] = phutil_tag('th', array());
} else {
$class = 'right';
$out[] = phutil_tag('th', array(), $p['oline']);
if ($left_prefix) {
$left_id = $left_prefix.$p['oline'];
} else {
$left_id = null;
}
$out[] = phutil_tag('th', array(), $p['line']);
$out[] = phutil_tag('th', array('id' => $left_id), $p['oline']);
}
if ($right_prefix) {
$right_id = $right_prefix.$p['line'];
} else {
$right_id = null;
}
$out[] = phutil_tag('th', array('id' => $right_id), $p['line']);
$out[] = phutil_tag('td', array('class' => $class), $p['render']);
}
$out[] = hsprintf('</tr>');

24
src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
View file

@ -55,19 +55,8 @@ final class DifferentialChangesetTwoUpRenderer
$new_lines = $this->getNewLines();
$gaps = $this->getGaps();
$reference = $this->getRenderingReference();
$left_id = $this->getOldChangesetID();
$right_id = $this->getNewChangesetID();
// "N" stands for 'new' and means the comment should attach to the new file
// when stored, i.e. DifferentialInlineComment->setIsNewFile().
// "O" stands for 'old' and means the comment should attach to the old file.
$left_char = $this->getOldAttachesToNewFile()
? 'N'
: 'O';
$right_char = $this->getNewAttachesToNewFile()
? 'N'
: 'O';
list($left_prefix, $right_prefix) = $this->getLineIDPrefixes();
$changeset = $this->getChangeset();
$copy_lines = idx($changeset->getMetadata(), 'copy:lines', array());
@ -234,14 +223,14 @@ final class DifferentialChangesetTwoUpRenderer
$html[] = $context_not_available;
}
if ($o_num && $left_id) {
$o_id = 'C'.$left_id.$left_char.'L'.$o_num;
if ($o_num && $left_prefix) {
$o_id = $left_prefix.$o_num;
} else {
$o_id = null;
}
if ($n_num && $right_id) {
$n_id = 'C'.$right_id.$right_char.'L'.$n_num;
if ($n_num && $right_prefix) {
$n_id = $right_prefix.$n_num;
} else {
$n_id = null;
}
@ -251,9 +240,6 @@ final class DifferentialChangesetTwoUpRenderer
// clipboard. See the 'phabricator-oncopy' behavior.
$zero_space = "\xE2\x80\x8B";
// NOTE: The Javascript is sensitive to whitespace changes in this
// block!
$html[] = phutil_tag('tr', array(), array(
phutil_tag('th', array('id' => $o_id), $o_num),
phutil_tag('td', array('class' => $o_classes), $o_text),