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 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(''); 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),