[epriest]

This is not (and has never been) the behavior of Phabricator.

See <https://secure.phabricator.com/T7447> for discussion of why this feature can never work the way you think it should work in the general case and why I believe other implementations, particularly GitHub's implementation, make the wrong tradeoffs (GitHub simply discards comments it can't find an exact matching line for).

If you believe this feature is possible to implement the way you imagine, I invite you to suggest an implementation. I am confident I can easily provide a counterexample which your implementation gets wrong (by either porting the inline forward to a line a human user would not choose, or by failing to port an inline which is still relevant forward).

[kevincox]

I don't need perfect, I just need good. GitHub and GitLab both have good implementations as well as every other good code review system I have used. GitHub annoying tries its hardest to hide the "outdated" comments but GitLab has the option to keep them open (they are no longer visible in the code, but remain on the discussion tab)

So I appreciate your opinion that it is impossible, but as a reviewer I much prefer when the tool tries.

[epriest]

GitHub's implementation does not do what you claim it does. GitHub has no behavior around porting and placing comments (while Phabricator does), GitHub just hides anything it can't place exactly. See my link above for a detailed description of GitHub's very simple implementation. I believe this is absolutely the wrong tradeoff.

[kevincox]

I use GitHub every day, I've definitely seen it preserve some comments. Sure, it drops a lot. But I still prefer this to dropping them all, or showing the comments on the wrong lines.

[epriest]

I mean that GitHub does not "try", in the sense of looking at the interdiff, doing fuzzy matching, trying to identify line-by-line similarity, etc. It places comments only if the hunk is exactly unchanged and gives up otherwise.

Phabricator does "try", in the sense that it examines the interdiff and attempts (of course, imperfectly, because no implementation can be perfect) to track line movement across hunk mutations.

My claim is that all comments which GitHub places correctly, Phabricator also places correctly. And some comments which GitHub drops, Phabricator places correctly (on the same line a human would select)! However, some comments which GitHub drops, Phabricator places incorrectly (on a line other than the line a human would select).

So the actual implementation you prefer is not one that tries, but one that doesn't try! Phabricator could have approximately GitHub's behavior by just deleting a bunch of code.

That's perfectly fine: many other users also prefer comments be discarded rather than tracked to a possibly-wrong line, too. I strongly believe this isn't a good behavior for code review software, which is why Phabricator doesn't do it -- but Phabricator puts substantially more effort into trying to track and place comments correctly than GitHub does.

[epriest]

In particular, see <https://secure.phabricator.com/T7447#112231> for a specific example which I believe GitHub's implementation gets egregiously wrong, by silently discarding an inline which is highly relevant to discussing the change.