Page MenuHomePhabricator
Files F15379800

D11983.diff
No OneTemporary
Actions

D11983.diff
View Options

diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php
@@ -525,5 +525,48 @@
$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);
+ }
}
diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php
@@ -30,6 +30,8 @@
$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 @@
} 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('id' => $left_id), $p['oline']);
}
- $out[] = phutil_tag('th', array(), $p['line']);
+
+ 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>');
diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
@@ -55,19 +55,8 @@
$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 @@
$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 @@
// 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),

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 14, 11:27 PM (3 w, 1 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7223292
Default Alt Text
D11983.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility