recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

When replying to a ghost comment, attach the reply to the same place

Summary:
Fixes T10562. I left this behavior sort of ambiguous in the original implementation because I didn't anticipate or stumble across this situation.

It's easy to fix: when you reply to a ghost, just put the reply in the exact same place as the ghost (even if it's a different diff), so they always move/ghost/port/thread together.

Test Plan:
See T10562 for reproduction steps and a "before" picture. Here's the after picture:

{F1168983}

The two comments at the bottom are pre-fix, and exhibit the bug. The comment at the top is post-fix, and appears adjacent to the original correctly.

Reviewers: chad

Reviewed By: chad

Subscribers: eadler

Maniphest Tasks: T10562

Differential Revision: https://secure.phabricator.com/D15458

epriestley 8858b6cf 7b8da999

+10 -5
+10 -5
src/infrastructure/diff/PhabricatorInlineCommentController.php
··· 305 305 pht('Failed to load comment "%s".', $reply_phid)); 306 306 } 307 307 308 - // NOTE: It's fine to reply to a comment from a different changeset, so 309 - // the reply comment may not appear on the same changeset that the new 310 - // comment appears on. This is expected in the case of ghost comments. 311 - // We currently put the new comment on the visible changeset, not the 312 - // original comment's changeset. 308 + // When replying, force the new comment into the same location as the 309 + // old comment. If we don't do this, replying to a ghost comment from 310 + // diff A while viewing diff B can end up placing the two comments in 311 + // different places while viewing diff C, because the porting algorithm 312 + // makes a different decision. Forcing the comments to bind to the same 313 + // place makes sure they stick together no matter which diff is being 314 + // viewed. See T10562 for discussion. 313 315 316 + $this->changesetID = $reply_comment->getChangesetID(); 314 317 $this->isNewFile = $reply_comment->getIsNewFile(); 318 + $this->lineNumber = $reply_comment->getLineNumber(); 319 + $this->lineLength = $reply_comment->getLineLength(); 315 320 } 316 321 } 317 322