Improve rendering readability

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

lua/octo/gh/graphql.lua

@@ -1536,7 +1536,7 @@ query($endCursor: String) {
       closedAt
       updatedAt
       url
-      repository { nameWithOwner }
+      headRepository { nameWithOwner }
       files(first:100) {
         nodes {
           path

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/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),

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 {

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

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)

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