diff --git a/README.md b/README.md index b844dbd2b..0778437a4 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,7 @@ require"octo".setup({ }, reviews = { auto_show_threads = true, -- automatically show comment threads on cursor move + focus = "right", -- focus right buffer on diff open }, pull_requests = { order_by = { -- criteria to sort the results of `Octo pr list` diff --git a/lua/octo/config.lua b/lua/octo/config.lua index e80ae29ae..6ef316fbd 100644 --- a/lua/octo/config.lua +++ b/lua/octo/config.lua @@ -4,6 +4,7 @@ local M = {} ---@alias OctoMappingsWindow "issue" | "pull_request" | "review_thread" | "submit_win" | "review_diff" | "file_panel" | "repo" ---@alias OctoMappingsList { [string]: table} ---@alias OctoPickers "telescope" | "fzf-lua" +---@alias OctoFocus "right" | "left" ---@class OctoPickerConfig ---@field use_emojis boolean @@ -37,6 +38,7 @@ local M = {} ---@class OctoConfigReviews ---@field auto_show_threads boolean +---@field focus OctoFocus ---@class OctoConfigPR ---@field order_by OctoConfigOrderBy @@ -134,6 +136,7 @@ function M.get_default_values() }, reviews = { auto_show_threads = true, + focus = "right", }, pull_requests = { order_by = { @@ -304,7 +307,7 @@ function M.validate_config() errors[value] = msg end - ---Checks if a variable is the correct, type if not it calls err with an error string + ---Checks if a variable is the correct type if not it calls err with an error string ---@param value any ---@param name string ---@param expected_types type | type[] @@ -332,21 +335,30 @@ function M.validate_config() return true end - local function validate_pickers() - local valid_pickers = { "telescope", "fzf-lua" } - if not validate_type(config.picker, "picker", "string") then - return - end - if not vim.tbl_contains(valid_pickers, config.picker) then - err( - "picker." .. config.picker, - string.format( - "Expected a valid picker, received '%s', which is not a supported picker! Valid pickers: ", - config.picker, - table.concat(valid_pickers, ", ") + ---Checks if a variable is one of the allowed string value + ---@param value any + ---@param name string + ---@param expected_strings string[] + local function validate_string_enum(value, name, expected_strings) + -- First check that the value is indeed a string + if validate_type(value, name, "string") then + -- Then check it matches one of the expected values + if not vim.tbl_contains(expected_strings, value) then + err( + name .. "." .. value, + string.format( + "Received '%s', which is not supported! Valid values: %s", + value, + table.concat(expected_strings, ", ") + ) ) - ) + end end + end + + local function validate_pickers() + validate_string_enum(config.picker, "picker", { "telescope", "fzf-lua" }) + if not validate_type(config.picker_config, "picker_config", "table") then return end @@ -355,25 +367,6 @@ function M.validate_config() validate_type(config.picker_config.mappings, "picker_config.mappings", "table") end - local function validate_user_search() - if not validate_type(config.users, "users", "string") then - return - end - - local valid_finders = { "search", "mentionable", "assignable" } - - if not vim.tbl_contains(valid_finders, config.users) then - err( - "users." .. config.users, - string.format( - "Expected a valid user finder, received '%s', which is not a supported finder! Valid finders: %s", - config.users, - table.concat(valid_finders, ", ") - ) - ) - end - end - local function validate_aliases() if not validate_type(config.ssh_aliases, "ssh_aliases", "table") then return @@ -393,6 +386,15 @@ function M.validate_config() end end + local function validate_reviews() + if not validate_type(config.reviews, "reviews", "table") then + return + end + + validate_type(config.reviews.auto_show_threads, "reviews.auto_show_threads", "boolean") + validate_string_enum(config.reviews.focus, "reviews.focus", { "right", "left" }) + end + local function validate_pull_requests() if not validate_type(config.pull_requests, "pull_requests", "table") then return @@ -423,7 +425,7 @@ function M.validate_config() validate_type(config.gh_cmd, "gh_cmd", "string") validate_type(config.gh_env, "gh_env", { "table", "function" }) validate_type(config.reaction_viewer_hint_icon, "reaction_viewer_hint_icon", "string") - validate_user_search() + validate_string_enum(config.users, "users", { "search", "mentionable", "assignable" }) validate_type(config.user_icon, "user_icon", "string") validate_type(config.comment_icon, "comment_icon", "string") validate_type(config.outdated_icon, "outdated_icon", "string") @@ -451,6 +453,7 @@ function M.validate_config() end validate_issues() + validate_reviews() validate_pull_requests() if validate_type(config.file_panel, "file_panel", "table") then validate_type(config.file_panel.size, "file_panel.size", "number") diff --git a/lua/octo/gh/graphql.lua b/lua/octo/gh/graphql.lua index 4099fed4d..ab27bc989 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/mappings.lua b/lua/octo/mappings.lua index 68b57ee81..bfbde03d6 100644 --- a/lua/octo/mappings.lua +++ b/lua/octo/mappings.lua @@ -1,4 +1,5 @@ local reviews = require "octo.reviews" +local config = require "octo.config" return { close_issue = function() @@ -145,7 +146,7 @@ return { local file_idx = layout.file_idx % #layout.files + 1 local file = layout.files[file_idx] if file then - layout:set_file(file, true) + layout:set_file(file, config.values.reviews.focus) end end end, @@ -155,7 +156,7 @@ return { local file_idx = (layout.file_idx - 2) % #layout.files + 1 local file = layout.files[file_idx] if file then - layout:set_file(file, true) + layout:set_file(file, config.values.reviews.focus) end end end, @@ -164,7 +165,7 @@ return { if layout and layout.file_panel:is_open() then local file = layout.files[1] if file then - layout:set_file(file, true) + layout:set_file(file, config.values.reviews.focus) end end end, @@ -173,7 +174,7 @@ return { if layout and layout.file_panel:is_open() then local file = layout.files[#layout.files] if file then - layout:set_file(file, true) + layout:set_file(file, config.values.reviews.focus) end end end, @@ -194,7 +195,7 @@ return { if layout and layout.file_panel:is_open() then local file = layout.file_panel:get_file_at_cursor() if file then - layout:set_file(file, true) + layout:set_file(file, config.values.reviews.focus) end end end, @@ -217,7 +218,7 @@ return { end end, close_review_win = function() - vim.api.nvim_win_close(vim.api.nvim_get_current_win()) + vim.api.nvim_win_close(vim.api.nvim_get_current_win(), true) end, approve_review = function() local current_review = reviews.get_current_review() 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 e74e83a2e..20bd290c3 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 { @@ -275,12 +275,15 @@ end function FileEntry:show_diff() for _, bufid in ipairs { self.left_bufid, self.right_bufid } do vim.api.nvim_buf_call(bufid, function() - pcall(vim.cmd, [[filetype detect]]) - pcall(vim.cmd, [[doau BufEnter]]) - pcall(vim.cmd, [[diffthis]]) + -- Only trigger ft detect event for non local files to avoid triggering ftplugins for nothing + if vim.fn.bufname(bufid):match "octo://*" then + pcall(vim.cmd.filetype, [[detect]]) + end + pcall(vim.cmd.doau, [[BufEnter]]) + pcall(vim.cmd.diffthis) -- Scroll to trigger the scrollbind and sync the windows. This works more -- consistently than calling `:syncbind`. - pcall(vim.cmd, [[exec "normal! \"]]) + pcall(vim.cmd.exec, [["normal! \"]]) end) end end diff --git a/lua/octo/reviews/init.lua b/lua/octo/reviews/init.lua index e9ae1eb32..84b70deaa 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) @@ -518,41 +518,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/reviews/layout.lua b/lua/octo/reviews/layout.lua index 20f46a255..b8c809229 100644 --- a/lua/octo/reviews/layout.lua +++ b/lua/octo/reviews/layout.lua @@ -4,6 +4,7 @@ local FilePanel = require("octo.reviews.file-panel").FilePanel local utils = require "octo.utils" local file_entry = require "octo.reviews.file-entry" +local config = require "octo.config" local M = {} @@ -51,7 +52,7 @@ function Layout:open(review) local file = self:cur_file() if file then - self:set_file(file) + self:set_file(file, config.values.reviews.focus) else self:file_safeguard() end @@ -133,7 +134,7 @@ function Layout:update_files() self.file_panel:render() self.file_panel:redraw() local file = self:cur_file() - self:set_file(file) + self:set_file(file, config.values.reviews.focus) self.update_needed = false end diff --git a/lua/octo/ui/writers.lua b/lua/octo/ui/writers.lua index 7c91f2b0f..70b576e6c 100644 --- a/lua/octo/ui/writers.lua +++ b/lua/octo/ui/writers.lua @@ -17,7 +17,7 @@ function M.write_block(bufnr, lines, line, mark) mark = mark or false if type(lines) == "string" then - lines = vim.split(lines, "\n", true) + lines = vim.split(lines, "\n") end -- write content lines @@ -45,6 +45,16 @@ function M.write_block(bufnr, lines, line, mark) end end + local indent_text = string.rep(" ", 2 * 4) + for i = start_line, end_line do + vim.api.nvim_buf_set_extmark(bufnr, constants.OCTO_COMMENT_NS, i - 1, 0, { + virt_text = { { indent_text, "Whitespace" } }, + virt_text_pos = "inline", + virt_text_repeat_linebreak = true, + right_gravity = false, + }) + end + return vim.api.nvim_buf_set_extmark(bufnr, constants.OCTO_COMMENT_NS, math.max(0, start_line - 1 - 1), 0, { end_line = math.min(end_line + 2 - 1, vim.api.nvim_buf_line_count(bufnr)), end_col = 0, @@ -595,9 +605,20 @@ function M.write_comment(bufnr, comment, kind, line) if vim.startswith(comment_body, constants.NO_BODY_MSG) or utils.is_blank(comment_body) then comment_body = " " end - local content = vim.split(comment_body, "\n", true) + local content = vim.split(comment_body, "\n") vim.list_extend(content, { "" }) + + -- Set extmark with virtual text at the beginning of the line local comment_mark = M.write_block(bufnr, content, line, true) + -- + -- -- Define the number of spaces for indentation (e.g., 4 spaces per indent level) + -- local indent_text = string.rep(" ", 2 * 4) -- Adjust 4 based on your desired indent width + -- vim.api.nvim_buf_set_extmark(bufnr, comment_vt_ns, line - 1, 0, { + -- virt_lines = { { { "foo" .. indent_text, "Normal" } }, { { "bar" .. indent_text, "Normal" } } }, + -- virt_lines_leftcol = true, + -- end_row = line + 1, + -- virt_text_pos = "inline", + -- }) line = line + #content diff --git a/lua/octo/utils.lua b/lua/octo/utils.lua index 3a8b08f6e..b07ae5737 100644 --- a/lua/octo/utils.lua +++ b/lua/octo/utils.lua @@ -191,10 +191,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 @@ -210,12 +213,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) @@ -274,37 +277,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 @@ -1130,8 +1118,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 @@ -1145,15 +1133,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) @@ -1165,9 +1153,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,