Skip to content

Commit

Permalink
fix: Improved more accurate diffing strategy
Browse files Browse the repository at this point in the history
Previously source was rebased on target, then check diff to target;
this is not how ADO does things, and it leads to incorrect comment
positions.

Now we get the common ancestor commit, and do the diff to that commit.
No more rebasing is required.
  • Loading branch information
Willem Jan Noort committed Jul 10, 2024
1 parent 3b17bd9 commit 24ab36a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 27 deletions.
4 changes: 2 additions & 2 deletions lua/adopure/activate.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ local function confirm_open_in_diffview(pull_request)
if not input then
return
end
local remote_target_name = "origin/" .. vim.split(pull_request.targetRefName, "refs/heads/")[2]
vim.cmd(":DiffviewOpen " .. remote_target_name)
local merge_base = require("adopure.git").get_merge_base(pull_request)
vim.cmd(":DiffviewOpen " .. merge_base)
end)
end

Expand Down
48 changes: 23 additions & 25 deletions lua/adopure/git.lua
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,28 @@ function M.get_remote_config()
return extract_git_details(elected_remote)
end

---Get merge base commit
---@param pull_request adopure.PullRequest
---@return string merge_base
function M.get_merge_base(pull_request)
local get_merge_base = require("plenary.job"):new({
command = "git",
args = { "merge-base", pull_request.lastMergeSourceCommit.commitId, pull_request.lastMergeTargetCommit.commitId },
cwd = ".",
})
get_merge_base:start()
local result = require("adopure.utils").await_result(get_merge_base)
if result.stderr[1] then
if not result.stdout[1] then
error(result.stderr[1])
end
vim.notify(result.stderr[1], 3)
end
assert(result.stdout[1], "No merge base found;")
return result.stdout[1]
end


---@param pull_request adopure.PullRequest
---@param open_callable function
function M.confirm_checkout_and_open(pull_request, open_callable)
Expand All @@ -76,11 +98,10 @@ function M.confirm_checkout_and_open(pull_request, open_callable)
open_callable()
return
end
local remote_source_name = "origin/" .. vim.split(pull_request.sourceRefName, "refs/heads/")[2]

local git_checkout_remote_job = Job:new({
command = "git",
args = { "checkout", remote_source_name },
args = { "checkout", pull_request.lastMergeSourceCommit.commitId },
cwd = ".",
on_exit = function(j, return_val)
if return_val ~= 0 then
Expand All @@ -89,29 +110,6 @@ function M.confirm_checkout_and_open(pull_request, open_callable)
end,
})

local remote_target_name = "origin/" .. vim.split(pull_request.targetRefName, "refs/heads/")[2]
local git_rebase_abort_job = Job:new({
command = "git",
args = { "rebase", "--abort" },
cwd = ".",
on_exit = function(j, return_val)
if return_val ~= 0 then
error("Rebase abort failed: " .. vim.inspect(j:result()))
end
end,
})
local git_rebase_job = Job:new({
command = "git",
args = { "rebase", remote_target_name },
cwd = ".",
on_exit = function(j, return_val)
if return_val ~= 0 then
git_rebase_abort_job:start()
error("Rebase failed: " .. vim.inspect(j:result()))
end
end,
})
git_checkout_remote_job:and_then_on_success(git_rebase_job)
git_checkout_remote_job:start()
open_callable()
end)
Expand Down
6 changes: 6 additions & 0 deletions lua/adopure/types.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@
---@field createdBy adopure.User
---@field creationDate string
---@field mergeStatus string
---@field lastMergeCommit MergeCommit
---@field lastMergeSourceCommit MergeCommit
---@field lastMergeTargetCommit MergeCommit
---@field reviewers adopure.Reviewer[]

---@class MergeCommit
---@field commitId string

---@class adopure.User
---@field displayName string
---@field id string
Expand Down

0 comments on commit 24ab36a

Please sign in to comment.