diff --git a/README.md b/README.md index 95e93d797..c51f5b732 100644 --- a/README.md +++ b/README.md @@ -439,8 +439,7 @@ Octo search assignee:pwntester is:pr ## 📋 PR reviews -- Open the PR (e.g. `Octo ` or `Octo pr list` or `Octo pr edit `) -- Start a review with `Octo review start` or resume a pending review with `Octo review resume` +- Enter review mode for the current branch with `Octo review`. Alternatively open the PR (e.g. `Octo ` or `Octo pr list` or `Octo pr edit `) then use `Octo review` in the PR buffer to enter review mode for a specific PR. - A new tab will show a panel with changed files and two windows showing the diff on any of them. - Change panel entries with `]q` and `[q` or by selecting an entry in the window - Add comments with `ca` or suggestions with `sa` on single or multiple visual-selected lines diff --git a/lua/octo/gh/graphql.lua b/lua/octo/gh/graphql.lua index 64cc191f3..ed7d4a614 100644 --- a/lua/octo/gh/graphql.lua +++ b/lua/octo/gh/graphql.lua @@ -1536,7 +1536,7 @@ query($endCursor: String) { closedAt updatedAt url - repository { nameWithOwner } + headRepository { nameWithOwner } files(first:100) { nodes { path diff --git a/lua/octo/model/octo-buffer.lua b/lua/octo/model/octo-buffer.lua index 44352f2b0..ad477b965 100644 --- a/lua/octo/model/octo-buffer.lua +++ b/lua/octo/model/octo-buffer.lua @@ -908,6 +908,8 @@ function OctoBuffer:get_pr() return PullRequest:new { bufnr = bufnr, repo = self.repo, + head_repo = self.node.headRepository.nameWithOwner, + head_ref_name = self.node.headRefName, number = self.number, id = self.node.id, left = Rev:new(self.node.baseRefOid), diff --git a/lua/octo/model/pull-request.lua b/lua/octo/model/pull-request.lua index 1890ff7c4..32d0ec318 100644 --- a/lua/octo/model/pull-request.lua +++ b/lua/octo/model/pull-request.lua @@ -5,6 +5,8 @@ local M = {} ---@class PullRequest ---@field repo string +---@field head_repo string +---@field head_ref_name string ---@field owner string ---@field name string ---@field number integer @@ -23,8 +25,9 @@ PullRequest.__index = PullRequest ---@return PullRequest function PullRequest:new(opts) local this = { - -- TODO: rename to nwo repo = opts.repo, + head_repo = opts.head_repo, + head_ref_name = opts.head_ref_name, number = opts.number, owner = "", name = "", @@ -57,7 +60,8 @@ end M.PullRequest = PullRequest ----Fetch the diff of the PR +--- Fetch the diff of the PR +--- @param pr PullRequest function PullRequest:get_diff(pr) local url = string.format("repos/%s/pulls/%d", pr.repo, pr.number) gh.run { diff --git a/lua/octo/reviews/file-entry.lua b/lua/octo/reviews/file-entry.lua index 7d6d26516..c309e6288 100644 --- a/lua/octo/reviews/file-entry.lua +++ b/lua/octo/reviews/file-entry.lua @@ -238,7 +238,7 @@ function FileEntry:load_buffers(left_winid, right_winid) for _, split in ipairs(splits) do if not split.bufid or not vim.api.nvim_buf_is_loaded(split.bufid) then local conf = config.values - local use_local = conf.use_local_fs and split.pos == "right" and utils.in_pr_branch(self.pull_request.bufnr) + local use_local = conf.use_local_fs and split.pos == "right" and utils.in_pr_branch(self.pull_request) -- create buffer split.bufid = M._create_buffer { diff --git a/lua/octo/reviews/init.lua b/lua/octo/reviews/init.lua index b477e1a4d..b11089f52 100644 --- a/lua/octo/reviews/init.lua +++ b/lua/octo/reviews/init.lua @@ -152,7 +152,7 @@ function Review:initiate(opts) opts = opts or {} local pr = self.pull_request local conf = config.values - if conf.use_local_fs and not utils.in_pr_branch(pr.bufnr) then + if conf.use_local_fs and not utils.in_pr_branch(pr) then local choice = vim.fn.confirm("Currently not in PR branch, would you like to checkout?", "&Yes\n&No", 2) if choice == 1 then utils.checkout_pr_sync(pr.number) @@ -520,41 +520,43 @@ function M.close(tabpage) end end ---- Get the pull request associated with current buffer +--- Get the pull request associated with current buffer. +--- Fall back to pull request associated with the current branch --- @param cb function -local function get_pr_from_buffer(cb) +local function get_pr_from_buffer_or_current_branch(cb) local bufnr = vim.api.nvim_get_current_buf() local buffer = octo_buffers[bufnr] + if not buffer then - utils.error "No Octo buffer found" + -- We are not in an octo buffer, try and fallback to the current branch's pr + utils.get_pull_request_for_current_branch(cb) return end + local pull_request = buffer:get_pr() if pull_request then cb(pull_request) else - pull_request = utils.get_pull_request_for_current_branch(function(pr) - cb(pr) - end) + pull_request = utils.get_pull_request_for_current_branch(cb) end end function M.start_review() - get_pr_from_buffer(function(pull_request) + get_pr_from_buffer_or_current_branch(function(pull_request) local current_review = Review:new(pull_request) current_review:start() end) end function M.resume_review() - get_pr_from_buffer(function(pull_request) + get_pr_from_buffer_or_current_branch(function(pull_request) local current_review = Review:new(pull_request) current_review:resume() end) end function M.start_or_resume_review() - get_pr_from_buffer(function(pull_request) + get_pr_from_buffer_or_current_branch(function(pull_request) local current_review = Review:new(pull_request) current_review:start_or_resume() end) diff --git a/lua/octo/utils.lua b/lua/octo/utils.lua index 8a7c68b54..3b035076c 100644 --- a/lua/octo/utils.lua +++ b/lua/octo/utils.lua @@ -254,10 +254,13 @@ function M.parse_git_remote() return remotes end -function M.get_remote() +--- Returns first host and repo information found in a list of remote values +--- If no argument is provided, defaults to matching against config's default remote +--- @param remote table | nil list of local remotes to match against +function M.get_remote(remote) local conf = config.values local remotes = M.parse_git_remote() - for _, name in ipairs(conf.default_remote) do + for _, name in ipairs(remote or conf.default_remote) do if remotes[name] then return remotes[name] end @@ -283,12 +286,12 @@ function M.get_all_remotes() return vim.tbl_values(M.parse_git_remote()) end -function M.get_remote_name() - return M.get_remote().repo +function M.get_remote_name(remote) + return M.get_remote(remote).repo end -function M.get_remote_host() - return M.get_remote().host +function M.get_remote_host(remote) + return M.get_remote(remote).host end function M.commit_exists(commit, cb) @@ -363,37 +366,22 @@ function M.in_pr_repo() end end -function M.in_pr_branch(bufnr) - bufnr = bufnr or vim.api.nvim_get_current_buf() - local buffer = octo_buffers[bufnr] - if not buffer then - return - end - if not buffer:isPullRequest() then - --M.error("Not in Octo PR buffer") - return false - end +--- Determines if we are locally are in a branch matching the pr head ref +--- @param pr PullRequest +--- @return boolean +function M.in_pr_branch(pr) + local cmd = "git rev-parse --abbrev-ref --symbolic-full-name @{u}" + local local_branch_with_local_remote = vim.split(string.gsub(vim.fn.system(cmd), "%s+", ""), "/") + local local_remote = local_branch_with_local_remote[1] + local local_branch = local_branch_with_local_remote[2] - local cmd = "git rev-parse --abbrev-ref HEAD" - local local_branch = string.gsub(vim.fn.system(cmd), "%s+", "") - if local_branch == string.format("%s/%s", buffer.node.headRepoName, buffer.node.headRefName) then - -- for PRs submitted from master, local_branch will get something like other_repo/master - local_branch = vim.split(local_branch, "/")[2] - end + local local_repo = M.get_remote_name { local_remote } - local local_repo = M.get_remote_name() - if buffer.node.baseRepository.nameWithOwner == local_repo and buffer.node.headRefName == local_branch then + if local_repo == pr.head_repo and local_branch == pr.head_ref_name then return true - elseif buffer.node.baseRepository.nameWithOwner ~= local_repo then - --M.error(string.format("Not in PR repo. Expected %s, got %s", buffer.node.baseRepository.nameWithOwner, local_repo)) - return false - elseif buffer.node.headRefName ~= local_branch then - -- TODO: suggest to checkout the branch - --M.error(string.format("Not in PR branch. Expected %s, got %s", buffer.node.headRefName, local_branch)) - return false - else - return false end + + return false end ---Checks out a PR b number @@ -1249,8 +1237,8 @@ function M.get_pull_request_for_current_branch(cb) return end local pr = vim.fn.json_decode(out) - local owner - local name + local base_owner + local base_name if pr.number then if pr.isCrossRepository then -- Parsing the pr url is the only way to get the target repo owner if the pr is cross repo @@ -1264,15 +1252,15 @@ function M.get_pull_request_for_current_branch(cb) return end local iter = url_suffix:gmatch "[^/]+/" - owner = vim.print(iter():sub(1, -2)) - name = vim.print(iter():sub(1, -2)) + base_owner = iter():sub(1, -2) + base_name = iter():sub(1, -2) else - owner = pr.headRepositoryOwner.login - name = pr.headRepository.name + base_owner = pr.headRepositoryOwner.login + base_name = pr.headRepository.name end local number = pr.number local id = pr.id - local query = graphql("pull_request_query", owner, name, number, _G.octo_pv2_fragment) + local query = graphql("pull_request_query", base_owner, base_name, number, _G.octo_pv2_fragment) gh.run { args = { "api", "graphql", "--paginate", "--jq", ".", "-f", string.format("query=%s", query) }, cb = function(output, stderr) @@ -1284,9 +1272,11 @@ function M.get_pull_request_for_current_branch(cb) local Rev = require("octo.reviews.rev").Rev local PullRequest = require("octo.model.pull-request").PullRequest local pull_request = PullRequest:new { - repo = owner .. "/" .. name, + repo = base_owner .. "/" .. base_name, + head_repo = obj.headRepository.nameWithOwner, number = number, id = id, + head_ref_name = obj.headRefName, left = Rev:new(obj.baseRefOid), right = Rev:new(obj.headRefOid), files = obj.files.nodes,