Merge pull request #618 from GuillaumeLagrange/make-review-focus-conf…

…igurable

Add config option to chose which buffer to focus on diff open

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`

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")

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()

lua/octo/reviews/file-entry.lua

@@ -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! \<c-y>"]])
+      pcall(vim.cmd.exec, [["normal! \<c-y>"]])
     end)
   end
 end

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