Skip to content

Comment on any expanded diff line on MRs

Oswaldo Ferreira requested to merge osw-comment-on-any-line-on-diffs into master

What does this MR do?

Allow commenting on any expanded diff lines on merge requests by fetching and inserting the content from blobs. In general, expanded diff lines will be treated as if it was part of the diff.

Screen_Shot_2018-10-16_at_13.24.32

Screen_Shot_2018-10-16_at_13.24.10

TODOs

  • Create a Diff::Expander class which receives blob, diff and position information. This class responsibility is to merge blob data with diff data given a position. The position will be used to determine if it's a expanded position, then give us the since and to information, which will be used to fetch the correct blob lines.
  • When sending a diff comment (DiffNote), we'll use the Diff::Expander#diff_lines to determine how the new diff should look like (and we'll be able to fetch the diff_line in the process).
  • backend Improve the logic to catch when the position doesn't need an expansion at all
  • backend Fix the < 20 expanded lines bug
  • backend Address top diff expansion comment
  • backend Address bottom diff expansion comment
  • backend Address highlighting cache (possibly by refreshing upon note creation)
  • backend Adjust reindex logic after blob merge with diff
  • frontend Fix ... request params when commenting in a expanded line, reloading and clicking ...
  • Fix commenting on diff additions
  • Fix parallel diff lines having no bottom match line
  • Discover reason for }} duplication on highlighted diffs
  • Refactor the whole thing
  • Add test for different comment positions on top / middle / bottom
  • Address test failures

Obs: Cache being handled on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22674.

What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/13950

Does this MR meet the acceptance criteria?

Edited by 🤖 GitLab Bot 🤖

Merge request reports