From dbdfac1e072ef7d77513bf41cac2ee1a7d8678f3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 5 Aug 2020 12:56:04 -0700 Subject: [PATCH] Recover inline comments which are "adjusted" off the end of a diff Summary: See PHI1834. Currently, the inline adjustment engine can sometime "adjust" an inline off the end of a diff. If it does, we lay it out on an invalid display line here and never render it. Instead, make sure that layout never puts a comment on an invalid line, so the UI is robust against questionable decisions by the adjustment engine: no adjustment should be able to accidentally discard an inline. Test Plan: - Created a two diff revision, where Diffs 1 and 2 have "alphabet.txt" with A-Z on one line each. The file is unchanged across diffs; some other file is changed. - Added a comment to lines P-Z of Diff 1. - Before: comment is adjusted out of range on Diff 2 and not shown in the UI. - After: comment is still adjusted out of range internally, but now corrected into the display range and shown. Differential Revision: https://secure.phabricator.com/D21435 --- .../parser/DifferentialChangesetParser.php | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 3652d18a9a..ad62b2a1ef 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -1053,14 +1053,30 @@ final class DifferentialChangesetParser extends Phobject { $this->comments = id(new PHUIDiffInlineThreader()) ->reorderAndThreadCommments($this->comments); + $old_max_display = 1; + foreach ($this->old as $old) { + if (isset($old['line'])) { + $old_max_display = $old['line']; + } + } + + $new_max_display = 1; + foreach ($this->new as $new) { + if (isset($new['line'])) { + $new_max_display = $new['line']; + } + } + foreach ($this->comments as $comment) { - $final = $comment->getLineNumber() + - $comment->getLineLength(); - $final = max(1, $final); + $display_line = $comment->getLineNumber() + $comment->getLineLength(); + $display_line = max(1, $display_line); + if ($this->isCommentOnRightSideWhenDisplayed($comment)) { - $new_comments[$final][] = $comment; + $display_line = min($new_max_display, $display_line); + $new_comments[$display_line][] = $comment; } else { - $old_comments[$final][] = $comment; + $display_line = min($old_max_display, $display_line); + $old_comments[$display_line][] = $comment; } } }