From feb7d1400d866fc43d4e061e4107aae90e53bee1 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Thu, 12 Sep 2024 16:00:33 +0200 Subject: [PATCH 1/3] fix: do not trigger ftplugins for local fs --- lua/octo/reviews/file-entry.lua | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lua/octo/reviews/file-entry.lua b/lua/octo/reviews/file-entry.lua index 7d6d2651..c3463a29 100644 --- a/lua/octo/reviews/file-entry.lua +++ b/lua/octo/reviews/file-entry.lua @@ -275,7 +275,10 @@ 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]]) + -- 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 From cd92ee2fdc7e68108d80f0b47f708f0a3dffeac8 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Fri, 27 Sep 2024 22:01:12 +0200 Subject: [PATCH 2/3] feat: make review focus configurable --- README.md | 1 + lua/octo/config.lua | 71 +++++++++++++++++++------------------ lua/octo/mappings.lua | 13 +++---- lua/octo/reviews/layout.lua | 5 +-- 4 files changed, 48 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 95e93d79..2a8dfd87 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 ae2d0d15..e05532dd 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 @@ -136,6 +138,7 @@ function M.get_default_values() }, reviews = { auto_show_threads = true, + focus = "right", }, pull_requests = { order_by = { @@ -306,7 +309,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[] @@ -334,21 +337,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 @@ -357,25 +369,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 @@ -395,6 +388,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 @@ -425,7 +427,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") @@ -453,6 +455,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/mappings.lua b/lua/octo/mappings.lua index 68b57ee8..bfbde03d 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/reviews/layout.lua b/lua/octo/reviews/layout.lua index 20f46a25..b8c80922 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 From 00bbd1ec16d24b743c8aea1abe7913e640fb9d70 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Fri, 27 Sep 2024 22:15:57 +0200 Subject: [PATCH 3/3] chore: fix linter warning --- lua/octo/reviews/file-entry.lua | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lua/octo/reviews/file-entry.lua b/lua/octo/reviews/file-entry.lua index c3463a29..bece8c73 100644 --- a/lua/octo/reviews/file-entry.lua +++ b/lua/octo/reviews/file-entry.lua @@ -277,13 +277,13 @@ function FileEntry:show_diff() vim.api.nvim_buf_call(bufid, function() -- 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]]) + pcall(vim.cmd.filetype, [[detect]]) end - pcall(vim.cmd, [[doau BufEnter]]) - pcall(vim.cmd, [[diffthis]]) + 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