From 37991e352c29530c2de773abc31a40b8af3a70ba Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sat, 15 Jul 2023 13:33:59 +0200
Subject: [PATCH 01/10] feat: auto load popups

This allows less locations to edit when adding a new popup
---
 lua/neogit/popups.lua              | 17 --------
 lua/neogit/popups/echo/actions.lua |  7 +++
 lua/neogit/popups/echo/init.lua    | 22 ++++++++++
 lua/neogit/popups/help/actions.lua | 32 +++++++-------
 lua/neogit/popups/init.lua         | 32 ++++++++++++++
 lua/neogit/status.lua              | 69 ++++++++++++++++--------------
 6 files changed, 114 insertions(+), 65 deletions(-)
 delete mode 100644 lua/neogit/popups.lua
 create mode 100644 lua/neogit/popups/echo/actions.lua
 create mode 100644 lua/neogit/popups/echo/init.lua
 create mode 100644 lua/neogit/popups/init.lua

diff --git a/lua/neogit/popups.lua b/lua/neogit/popups.lua
deleted file mode 100644
index a23174eae..000000000
--- a/lua/neogit/popups.lua
+++ /dev/null
@@ -1,17 +0,0 @@
-return {
-  branch = require("neogit.popups.branch"),
-  cherry_pick = require("neogit.popups.cherry_pick"),
-  commit = require("neogit.popups.commit"),
-  diff = require("neogit.popups.diff"),
-  fetch = require("neogit.popups.fetch"),
-  help = require("neogit.popups.help"),
-  log = require("neogit.popups.log"),
-  merge = require("neogit.popups.merge"),
-  pull = require("neogit.popups.pull"),
-  push = require("neogit.popups.push"),
-  rebase = require("neogit.popups.rebase"),
-  remote = require("neogit.popups.remote"),
-  reset = require("neogit.popups.reset"),
-  revert = require("neogit.popups.revert"),
-  stash = require("neogit.popups.stash"),
-}
diff --git a/lua/neogit/popups/echo/actions.lua b/lua/neogit/popups/echo/actions.lua
new file mode 100644
index 000000000..7ccf98ba9
--- /dev/null
+++ b/lua/neogit/popups/echo/actions.lua
@@ -0,0 +1,7 @@
+local M = {}
+function M.echo(value)
+  local notification = require("neogit.lib.notification")
+  notification.create("Echo: " .. value)
+end
+
+return M
diff --git a/lua/neogit/popups/echo/init.lua b/lua/neogit/popups/echo/init.lua
new file mode 100644
index 000000000..39f29536e
--- /dev/null
+++ b/lua/neogit/popups/echo/init.lua
@@ -0,0 +1,22 @@
+local popup = require("neogit.lib.popup")
+local actions = require("neogit.popups.echo.actions")
+
+local M = {}
+
+function M.create(...)
+  local args = { ... }
+  local p = popup.builder():name("NeogitEchoPopup")
+  for k, v in ipairs(args) do
+    p:action(tostring(k), tostring(v), function()
+      actions.echo(v)
+    end)
+  end
+
+  local p = p:build()
+
+  p:show()
+
+  return p
+end
+
+return M
diff --git a/lua/neogit/popups/help/actions.lua b/lua/neogit/popups/help/actions.lua
index 9188a4874..3a50b5a4c 100644
--- a/lua/neogit/popups/help/actions.lua
+++ b/lua/neogit/popups/help/actions.lua
@@ -23,27 +23,25 @@ M.popups = function(env)
   local popups = require("neogit.popups")
 
   return present {
-    { "HelpPopup", "Help", popups.help.create },
-    { "DiffPopup", "Diff", popups.diff.create },
-    { "PullPopup", "Pull", popups.pull.create },
-    { "RebasePopup", "Rebase", popups.rebase.create },
-    { "MergePopup", "Merge", popups.merge.create },
-    { "PushPopup", "Push", popups.push.create },
-    { "CommitPopup", "Commit", popups.commit.create },
-    { "LogPopup", "Log", popups.log.create },
-    { "CherryPickPopup", "Apply", popups.cherry_pick.create },
-    { "BranchPopup", "Branch", popups.branch.create },
-    { "FetchPopup", "Fetch", popups.fetch.create },
-    { "ResetPopup", "Reset", popups.reset.create },
-    { "RevertPopup", "Revert", popups.revert.create },
-    { "RemotePopup", "Remote", popups.remote.create },
+    { "HelpPopup", "Help", popups.open("help") },
+    { "DiffPopup", "Diff", popups.open("diff") },
+    { "PullPopup", "Pull", popups.open("pull") },
+    { "RebasePopup", "Rebase", popups.open("rebase") },
+    { "MergePopup", "Merge", popups.open("merge") },
+    { "PushPopup", "Push", popups.open("push") },
+    { "CommitPopup", "Commit", popups.open("commit") },
+    { "LogPopup", "Log", popups.open("log") },
+    { "CherryPickPopup", "Apply", popups.open("cherry_pick") },
+    { "BranchPopup", "Branch", popups.open("branch") },
+    { "FetchPopup", "Fetch", popups.open("fetch") },
+    { "ResetPopup", "Reset", popups.open("reset") },
+    { "RevertPopup", "Revert", popups.open("revert") },
+    { "RemotePopup", "Remote", popups.open("remote") },
     { "InitRepo", "Init", require("neogit.lib.git").init.init_repo },
     {
       "StashPopup",
       "Stash",
-      function()
-        popups.stash.create(env.get_stash())
-      end,
+      popups.open("stash", env.get_stash),
     },
     {
       "CommandHistory",
diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
new file mode 100644
index 000000000..773b2f232
--- /dev/null
+++ b/lua/neogit/popups/init.lua
@@ -0,0 +1,32 @@
+local M = {}
+
+---@param name string
+--@param get_args nil|fun(): any
+--- Creates a curried function which will open the popup with the given name when called
+--- Extra arguments are supplied to popup.`create()`
+function M.open(name, get_args)
+  return function()
+    local ok, value = pcall(require, "neogit.popups." .. name)
+    if ok then
+      assert(value)
+      local args = {}
+
+      if get_args then
+        args = { get_args() }
+      end
+
+      value.create(table.unpack(args))
+    else
+      local notification = require("neogit.lib.notification")
+      notification.create(string.format("No such popup: %q", name), vim.log.levels.ERROR)
+    end
+  end
+end
+
+function M.test()
+  M.open("echo", function()
+    return "a", "b"
+  end)()
+end
+
+return M
diff --git a/lua/neogit/status.lua b/lua/neogit/status.lua
index 7181c3154..5b575a30f 100644
--- a/lua/neogit/status.lua
+++ b/lua/neogit/status.lua
@@ -922,6 +922,7 @@ end
 --- These needs to be a function to avoid a circular dependency
 --- between this module and the popup modules
 local cmd_func_map = function()
+  local popups = require("neogit.popups")
   return {
     ["Close"] = function()
       M.status_buffer:close()
@@ -1085,21 +1086,13 @@ local cmd_func_map = function()
         end
       end
     end),
+
     ["RefreshBuffer"] = function()
       dispatch_refresh(true)
     end,
-    ["HelpPopup"] = function()
-      local line = M.status_buffer:get_current_line()
 
-      require("neogit.popups.help").create {
-        get_stash = function()
-          return {
-            name = line[1]:match("^(stash@{%d+})"),
-          }
-        end,
-        use_magit_keybindings = config.values.use_magit_keybindings,
-      }
-    end,
+    -- INTEGRATIONS --
+
     ["DiffAtFile"] = function()
       if not config.ensure_integration("diffview") then
         return
@@ -1111,32 +1104,46 @@ local cmd_func_map = function()
         dv.open(section.name, item.name)
       end
     end,
-    ["DiffPopup"] = require("neogit.popups.diff").create,
-    ["PullPopup"] = require("neogit.popups.pull").create,
-    ["RebasePopup"] = function()
+
+    --- POPUPS ---
+
+    ["HelpPopup"] = popups.open("help", function()
       local line = M.status_buffer:get_current_line()
-      require("neogit.popups.rebase").create { line[1]:match("^(%x%x%x%x%x%x%x+)") }
-    end,
-    ["MergePopup"] = require("neogit.popups.merge").create,
-    ["PushPopup"] = require("neogit.popups.push").create,
-    ["CommitPopup"] = require("neogit.popups.commit").create,
-    ["LogPopup"] = require("neogit.popups.log").create,
+
+      return {
+        get_stash = function()
+          return {
+            name = line[1]:match("^(stash@{%d+})"),
+          }
+        end,
+        use_magit_keybindings = config.values.use_magit_keybindings,
+      }
+    end),
+    ["DiffPopup"] = popups.open("diff"),
+    ["PullPopup"] = popups.open("pull"),
+    ["RebasePopup"] = popups.open("rebase", function()
+      local line = M.status_buffer:get_current_line()
+      return { line[1]:match("^(%x%x%x%x%x%x%x+)") }
+    end),
+    ["MergePopup"] = popups.open("merge"),
+    ["PushPopup"] = popups.open("push"),
+    ["CommitPopup"] = popups.open("commit"),
+    ["LogPopup"] = popups.open("log"),
     ["CherryPickPopup"] = { "nv", a.void(cherry_pick), true },
-    ["StashPopup"] = function()
+    ["StashPopup"] = popups.open("stash", function()
       local line = M.status_buffer:get_current_line()
-
-      require("neogit.popups.stash").create {
+      return {
         name = line[1]:match("^(stash@{%d+})"),
       }
-    end,
-    ["RevertPopup"] = function()
+    end),
+    ["RevertPopup"] = popups.open("revert", function()
       local line = M.status_buffer:get_current_line()
-      require("neogit.popups.revert").create { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
-    end,
-    ["BranchPopup"] = require("neogit.popups.branch").create,
-    ["FetchPopup"] = require("neogit.popups.fetch").create,
-    ["RemotePopup"] = require("neogit.popups.remote").create,
-    ["ResetPopup"] = require("neogit.popups.reset").create,
+      return { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
+    end),
+    ["BranchPopup"] = popups.open("branch"),
+    ["FetchPopup"] = popups.open("fetch"),
+    ["RemotePopup"] = popups.open("remote"),
+    ["ResetPopup"] = popups.open("reset"),
   }
 end
 

From 2973302b2c9185daf7397a52c5e1aa31549cffbe Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sat, 15 Jul 2023 13:58:10 +0200
Subject: [PATCH 02/10] chore: move popup mappings to popups module

---
 lua/neogit/lib/buffer.lua           |  2 +-
 lua/neogit/lib/mappings_manager.lua | 35 ++++++++++-------
 lua/neogit/popups/init.lua          | 59 +++++++++++++++++++++++++++++
 lua/neogit/status.lua               | 50 ++++--------------------
 4 files changed, 89 insertions(+), 57 deletions(-)

diff --git a/lua/neogit/lib/buffer.lua b/lua/neogit/lib/buffer.lua
index b2ee917e7..455896d4b 100644
--- a/lua/neogit/lib/buffer.lua
+++ b/lua/neogit/lib/buffer.lua
@@ -9,7 +9,7 @@ local Ui = require("neogit.lib.ui")
 
 ---@class Buffer
 ---@field handle number
----@field mmanager any
+---@field mmanager MappingsManager
 ---@field ui Ui
 ---@field kind string
 local Buffer = {
diff --git a/lua/neogit/lib/mappings_manager.lua b/lua/neogit/lib/mappings_manager.lua
index e624cd87d..4c0b0cf9f 100644
--- a/lua/neogit/lib/mappings_manager.lua
+++ b/lua/neogit/lib/mappings_manager.lua
@@ -1,6 +1,17 @@
 local managers = {}
 
-local function invoke(id, map_id)
+---@alias Mapping string|function|MappingTable
+
+---@class MappingTable
+---@field [1] string mode
+---@field [2] string|function command
+---@field [3] boolean Escape visual mode
+
+---@class MappingsManager
+---@field mappings table<string, Mapping>
+local MappingsManager = {}
+
+function MappingsManager.invoke(id, map_id)
   local manager = managers[id]
   local k = manager.map_id_to_key[map_id]
   local mapping = manager.mappings[k]
@@ -12,11 +23,16 @@ local function invoke(id, map_id)
   end
 end
 
-local function build_call_string(id, k)
+function MappingsManager.build_call_string(id, k)
   return string.format([[<cmd>lua require 'neogit.lib.mappings_manager'.invoke(%d, %d)<CR>]], id, k)
 end
 
-local function new(id)
+function MappingsManager.delete(id)
+  managers[id] = nil
+end
+
+---@return MappingsManager
+function MappingsManager.new(id)
   local mappings = {}
   local map_id_to_key = {}
   local manager = {
@@ -26,7 +42,7 @@ local function new(id)
     register = function()
       for k, mapping in pairs(mappings) do
         local map_id = #map_id_to_key + 1
-        local f_call = build_call_string(id, map_id)
+        local f_call = MappingsManager.build_call_string(id, map_id)
         if type(mapping) == "table" then
           for _, m in pairs(vim.split(mapping[1], "")) do
             if type(mapping[2]) == "string" then
@@ -61,13 +77,4 @@ local function new(id)
   return manager
 end
 
-local function delete(id)
-  managers[id] = nil
-end
-
-return {
-  new = new,
-  build_call_string = build_call_string,
-  delete = delete,
-  invoke = invoke,
-}
+return MappingsManager
diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index 773b2f232..41b4bc4b6 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -23,6 +23,65 @@ function M.open(name, get_args)
   end
 end
 
+--- Returns an array useful for creating mappings for the available popups
+---@return table<string, Mapping>
+function M.mappings_table()
+  local config = require("neogit.config")
+  return {
+    ["HelpPopup"] = M.open("help", function()
+      local status = require("neogit.status")
+      local line = status.status_buffer:get_current_line()
+
+      return {
+        get_stash = function()
+          return {
+            name = line[1]:match("^(stash@{%d+})"),
+          }
+        end,
+        use_magit_keybindings = config.values.use_magit_keybindings,
+      }
+    end),
+    ["DiffPopup"] = M.open("diff"),
+    ["PullPopup"] = M.open("pull"),
+    ["RebasePopup"] = M.open("rebase", function()
+      local status = require("neogit.status")
+      local line = status.status_buffer:get_current_line()
+      return { line[1]:match("^(%x%x%x%x%x%x%x+)") }
+    end),
+    ["MergePopup"] = M.open("merge"),
+    ["PushPopup"] = M.open("push"),
+    ["CommitPopup"] = M.open("commit"),
+    ["LogPopup"] = M.open("log"),
+    ["CherryPickPopup"] = M.open("cherry_pick", function()
+      local selection = nil
+
+      if vim.api.nvim_get_mode().mode == "V" then
+        local status = require("neogit.status")
+        selection = status.get_selected_commits()
+      end
+
+      return { commits = selection }
+    end),
+    -- { "nv", a.void(cherry_pick), true },
+    ["StashPopup"] = M.open("stash", function()
+      local status = require("neogit.status")
+      local line = status.status_buffer:get_current_line()
+      return {
+        name = line[1]:match("^(stash@{%d+})"),
+      }
+    end),
+    ["RevertPopup"] = M.open("revert", function()
+      local status = require("neogit.status")
+      local line = status.status_buffer:get_current_line()
+      return { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
+    end),
+    ["BranchPopup"] = M.open("branch"),
+    ["FetchPopup"] = M.open("fetch"),
+    ["RemotePopup"] = M.open("remote"),
+    ["ResetPopup"] = M.open("reset"),
+  }
+end
+
 function M.test()
   M.open("echo", function()
     return "a", "b"
diff --git a/lua/neogit/status.lua b/lua/neogit/status.lua
index 5b575a30f..a096363a9 100644
--- a/lua/neogit/status.lua
+++ b/lua/neogit/status.lua
@@ -611,6 +611,8 @@ local function get_selected_commits()
   return items
 end
 
+M.get_selectet_commits = get_selected_commits
+
 ---Determines if selection contains multiple files
 ---@return boolean
 local function selection_spans_multiple_items_within_section()
@@ -922,8 +924,7 @@ end
 --- These needs to be a function to avoid a circular dependency
 --- between this module and the popup modules
 local cmd_func_map = function()
-  local popups = require("neogit.popups")
-  return {
+  local mappings = {
     ["Close"] = function()
       M.status_buffer:close()
     end,
@@ -1104,47 +1105,11 @@ local cmd_func_map = function()
         dv.open(section.name, item.name)
       end
     end,
-
-    --- POPUPS ---
-
-    ["HelpPopup"] = popups.open("help", function()
-      local line = M.status_buffer:get_current_line()
-
-      return {
-        get_stash = function()
-          return {
-            name = line[1]:match("^(stash@{%d+})"),
-          }
-        end,
-        use_magit_keybindings = config.values.use_magit_keybindings,
-      }
-    end),
-    ["DiffPopup"] = popups.open("diff"),
-    ["PullPopup"] = popups.open("pull"),
-    ["RebasePopup"] = popups.open("rebase", function()
-      local line = M.status_buffer:get_current_line()
-      return { line[1]:match("^(%x%x%x%x%x%x%x+)") }
-    end),
-    ["MergePopup"] = popups.open("merge"),
-    ["PushPopup"] = popups.open("push"),
-    ["CommitPopup"] = popups.open("commit"),
-    ["LogPopup"] = popups.open("log"),
-    ["CherryPickPopup"] = { "nv", a.void(cherry_pick), true },
-    ["StashPopup"] = popups.open("stash", function()
-      local line = M.status_buffer:get_current_line()
-      return {
-        name = line[1]:match("^(stash@{%d+})"),
-      }
-    end),
-    ["RevertPopup"] = popups.open("revert", function()
-      local line = M.status_buffer:get_current_line()
-      return { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
-    end),
-    ["BranchPopup"] = popups.open("branch"),
-    ["FetchPopup"] = popups.open("fetch"),
-    ["RemotePopup"] = popups.open("remote"),
-    ["ResetPopup"] = popups.open("reset"),
   }
+
+  local popups = require("neogit.popups")
+  --- Load the popups from the centralized popup file
+  return vim.tbl_extend("error", mappings, popups.mappings_table())
 end
 
 -- Sets decoration provider for buffer
@@ -1237,6 +1202,7 @@ function M.create(kind, cwd)
     name = "NeogitStatus",
     filetype = "NeogitStatus",
     kind = kind,
+    ---@param buffer Buffer
     initialize = function(buffer)
       logger.debug("[STATUS BUFFER]: Initializing...")
 

From f687001b369e3b2a98612cdfc8411990811fdded Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sat, 15 Jul 2023 14:13:36 +0200
Subject: [PATCH 03/10] feat: make `help` use same popups as status buffer

This ensures that when new popups are added they are also visible in
the help buffer and not forgotten, as well as working the same when
accessed
---
 lua/neogit/lib/mappings_manager.lua |   2 +-
 lua/neogit/popups/help/actions.lua  |  33 +++-----
 lua/neogit/popups/init.lua          | 124 +++++++++++++++++-----------
 lua/neogit/status.lua               |  10 ++-
 4 files changed, 95 insertions(+), 74 deletions(-)

diff --git a/lua/neogit/lib/mappings_manager.lua b/lua/neogit/lib/mappings_manager.lua
index 4c0b0cf9f..6f30155f7 100644
--- a/lua/neogit/lib/mappings_manager.lua
+++ b/lua/neogit/lib/mappings_manager.lua
@@ -4,7 +4,7 @@ local managers = {}
 
 ---@class MappingTable
 ---@field [1] string mode
----@field [2] string|function command
+---@field [2] string|function func
 ---@field [3] boolean Escape visual mode
 
 ---@class MappingsManager
diff --git a/lua/neogit/popups/help/actions.lua b/lua/neogit/popups/help/actions.lua
index 3a50b5a4c..4f9c3370b 100644
--- a/lua/neogit/popups/help/actions.lua
+++ b/lua/neogit/popups/help/actions.lua
@@ -9,6 +9,11 @@ local function present(commands)
   local presenter = util.map(commands, function(command)
     local cmd, name, fn = unpack(command)
 
+    --- Handle the longer table mapping form (mode, func, esc)
+    if type(fn) == "table" then
+      fn = fn[2]
+    end
+
     return { name = name, key = status_mappings[cmd], fn = fn }
   end)
 
@@ -22,35 +27,19 @@ end
 M.popups = function(env)
   local popups = require("neogit.popups")
 
-  return present {
-    { "HelpPopup", "Help", popups.open("help") },
-    { "DiffPopup", "Diff", popups.open("diff") },
-    { "PullPopup", "Pull", popups.open("pull") },
-    { "RebasePopup", "Rebase", popups.open("rebase") },
-    { "MergePopup", "Merge", popups.open("merge") },
-    { "PushPopup", "Push", popups.open("push") },
-    { "CommitPopup", "Commit", popups.open("commit") },
-    { "LogPopup", "Log", popups.open("log") },
-    { "CherryPickPopup", "Apply", popups.open("cherry_pick") },
-    { "BranchPopup", "Branch", popups.open("branch") },
-    { "FetchPopup", "Fetch", popups.open("fetch") },
-    { "ResetPopup", "Reset", popups.open("reset") },
-    { "RevertPopup", "Revert", popups.open("revert") },
-    { "RemotePopup", "Remote", popups.open("remote") },
-    { "InitRepo", "Init", require("neogit.lib.git").init.init_repo },
-    {
-      "StashPopup",
-      "Stash",
-      popups.open("stash", env.get_stash),
-    },
+  local items = vim.list_extend({
     {
+
       "CommandHistory",
       "History",
       function()
         require("neogit.buffers.git_command_history"):new():show()
       end,
     },
-  }
+    { "InitRepo", "Init", require("neogit.lib.git").init.init_repo },
+  }, popups.mappings_table())
+
+  return present(items)
 end
 
 M.actions = function()
diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index 41b4bc4b6..bd5f13b08 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -5,7 +5,8 @@ local M = {}
 --- Creates a curried function which will open the popup with the given name when called
 --- Extra arguments are supplied to popup.`create()`
 function M.open(name, get_args)
-  return function()
+  local async = require("plenary.async")
+  return async.void(function()
     local ok, value = pcall(require, "neogit.popups." .. name)
     if ok then
       assert(value)
@@ -20,7 +21,7 @@ function M.open(name, get_args)
       local notification = require("neogit.lib.notification")
       notification.create(string.format("No such popup: %q", name), vim.log.levels.ERROR)
     end
-  end
+  end)
 end
 
 --- Returns an array useful for creating mappings for the available popups
@@ -28,57 +29,80 @@ end
 function M.mappings_table()
   local config = require("neogit.config")
   return {
-    ["HelpPopup"] = M.open("help", function()
-      local status = require("neogit.status")
-      local line = status.status_buffer:get_current_line()
-
-      return {
-        get_stash = function()
-          return {
-            name = line[1]:match("^(stash@{%d+})"),
-          }
-        end,
-        use_magit_keybindings = config.values.use_magit_keybindings,
-      }
-    end),
-    ["DiffPopup"] = M.open("diff"),
-    ["PullPopup"] = M.open("pull"),
-    ["RebasePopup"] = M.open("rebase", function()
-      local status = require("neogit.status")
-      local line = status.status_buffer:get_current_line()
-      return { line[1]:match("^(%x%x%x%x%x%x%x+)") }
-    end),
-    ["MergePopup"] = M.open("merge"),
-    ["PushPopup"] = M.open("push"),
-    ["CommitPopup"] = M.open("commit"),
-    ["LogPopup"] = M.open("log"),
-    ["CherryPickPopup"] = M.open("cherry_pick", function()
-      local selection = nil
+    {
+      "HelpPopup",
+      "Help",
+      M.open("help", function()
+        local status = require("neogit.status")
+        local line = status.status_buffer:get_current_line()
 
-      if vim.api.nvim_get_mode().mode == "V" then
+        return {
+          get_stash = function()
+            return {
+              name = line[1]:match("^(stash@{%d+})"),
+            }
+          end,
+          use_magit_keybindings = config.values.use_magit_keybindings,
+        }
+      end),
+    },
+    { "DiffPopup", "Diff", M.open("diff") },
+    { "PullPopup", "Pull", M.open("pull") },
+    {
+      "RebasePopup",
+      "Rebase",
+      M.open("rebase", function()
         local status = require("neogit.status")
-        selection = status.get_selected_commits()
-      end
+        local line = status.status_buffer:get_current_line()
+        return { line[1]:match("^(%x%x%x%x%x%x%x+)") }
+      end),
+    },
+    { "MergePopup", "Merge", M.open("merge") },
+    { "PushPopup", "Push", M.open("push") },
+    { "CommitPopup", "Commit", M.open("commit") },
+    { "LogPopup", "Log", M.open("log") },
+    {
+      "CherryPickPopup",
+      "Cherry Pick",
+      {
+        "nv",
+        M.open("cherry_pick", function()
+          local selection = nil
 
-      return { commits = selection }
-    end),
-    -- { "nv", a.void(cherry_pick), true },
-    ["StashPopup"] = M.open("stash", function()
-      local status = require("neogit.status")
-      local line = status.status_buffer:get_current_line()
-      return {
-        name = line[1]:match("^(stash@{%d+})"),
-      }
-    end),
-    ["RevertPopup"] = M.open("revert", function()
-      local status = require("neogit.status")
-      local line = status.status_buffer:get_current_line()
-      return { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
-    end),
-    ["BranchPopup"] = M.open("branch"),
-    ["FetchPopup"] = M.open("fetch"),
-    ["RemotePopup"] = M.open("remote"),
-    ["ResetPopup"] = M.open("reset"),
+          if vim.api.nvim_get_mode().mode == "V" then
+            local status = require("neogit.status")
+            selection = status.get_selected_commits()
+          end
+
+          return { commits = selection }
+        end),
+        true,
+      },
+    },
+    {
+      "StashPopup",
+      "Stash",
+      M.open("stash", function()
+        local status = require("neogit.status")
+        local line = status.status_buffer:get_current_line()
+        return {
+          name = line[1]:match("^(stash@{%d+})"),
+        }
+      end),
+    },
+    {
+      "RevertPopup",
+      "Revert",
+      M.open("revert", function()
+        local status = require("neogit.status")
+        local line = status.status_buffer:get_current_line()
+        return { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
+      end),
+    },
+    { "BranchPopup", "Branch", M.open("branch") },
+    { "FetchPopup", "Fetch", M.open("fetch") },
+    { "RemotePopup", "Remote", M.open("remote") },
+    { "ResetPopup", "Reset", M.open("reset") },
   }
 end
 
diff --git a/lua/neogit/status.lua b/lua/neogit/status.lua
index a096363a9..f5e03145b 100644
--- a/lua/neogit/status.lua
+++ b/lua/neogit/status.lua
@@ -1109,7 +1109,15 @@ local cmd_func_map = function()
 
   local popups = require("neogit.popups")
   --- Load the popups from the centralized popup file
-  return vim.tbl_extend("error", mappings, popups.mappings_table())
+  for _, v in ipairs(popups.mappings_table()) do
+    --- { name, display_name, mapping }
+    if mappings[v[1]] then
+      error("Neogit: Mapping '" .. v[1] .. "' is already in use!")
+    end
+
+    mappings[v[1]] = v[3]
+  end
+  return mappings
 end
 
 -- Sets decoration provider for buffer

From 8732bf522c6dd78dca2b36d9bbd1b09d0e55152e Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sun, 16 Jul 2023 10:28:15 +0200
Subject: [PATCH 04/10] feat: async popup opener

---
 lua/neogit/lib/popup.lua           |  6 ++-
 lua/neogit/popups/help/actions.lua |  2 +-
 lua/neogit/popups/init.lua         | 62 +++++++++++++++---------------
 3 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/lua/neogit/lib/popup.lua b/lua/neogit/lib/popup.lua
index 69a092f6d..ce4b4214c 100644
--- a/lua/neogit/lib/popup.lua
+++ b/lua/neogit/lib/popup.lua
@@ -78,8 +78,10 @@ end
 
 -- Closes the popup buffer
 function M:close()
-  self.buffer:close()
-  self.buffer = nil
+  if self.buffer then
+    self.buffer:close()
+    self.buffer = nil
+  end
 end
 
 -- Determines the correct highlight group for a switch based on it's state.
diff --git a/lua/neogit/popups/help/actions.lua b/lua/neogit/popups/help/actions.lua
index 4f9c3370b..34ae089da 100644
--- a/lua/neogit/popups/help/actions.lua
+++ b/lua/neogit/popups/help/actions.lua
@@ -24,7 +24,7 @@ local function present(commands)
   return presenter
 end
 
-M.popups = function(env)
+M.popups = function()
   local popups = require("neogit.popups")
 
   local items = vim.list_extend({
diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index bd5f13b08..2af65f071 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -1,42 +1,41 @@
 local M = {}
 
 ---@param name string
---@param get_args nil|fun(): any
+---@param f nil|fun(create: fun(...)): any
 --- Creates a curried function which will open the popup with the given name when called
---- Extra arguments are supplied to popup.`create()`
-function M.open(name, get_args)
-  local async = require("plenary.async")
-  return async.void(function()
+function M.open(name, f)
+  f = f or function(f)
+    f()
+  end
+
+  return function()
     local ok, value = pcall(require, "neogit.popups." .. name)
     if ok then
       assert(value)
-      local args = {}
+      assert(value.create)
 
-      if get_args then
-        args = { get_args() }
-      end
-
-      value.create(table.unpack(args))
+      f(value.create)
     else
       local notification = require("neogit.lib.notification")
       notification.create(string.format("No such popup: %q", name), vim.log.levels.ERROR)
     end
-  end)
+  end
 end
 
 --- Returns an array useful for creating mappings for the available popups
 ---@return table<string, Mapping>
 function M.mappings_table()
   local config = require("neogit.config")
+  local async = require("plenary.async")
   return {
     {
       "HelpPopup",
       "Help",
-      M.open("help", function()
+      M.open("help", function(f)
         local status = require("neogit.status")
         local line = status.status_buffer:get_current_line()
 
-        return {
+        f {
           get_stash = function()
             return {
               name = line[1]:match("^(stash@{%d+})"),
@@ -51,10 +50,10 @@ function M.mappings_table()
     {
       "RebasePopup",
       "Rebase",
-      M.open("rebase", function()
+      M.open("rebase", function(f)
         local status = require("neogit.status")
         local line = status.status_buffer:get_current_line()
-        return { line[1]:match("^(%x%x%x%x%x%x%x+)") }
+        f { line[1]:match("^(%x%x%x%x%x%x%x+)") }
       end),
     },
     { "MergePopup", "Merge", M.open("merge") },
@@ -66,26 +65,29 @@ function M.mappings_table()
       "Cherry Pick",
       {
         "nv",
-        M.open("cherry_pick", function()
-          local selection = nil
+        M.open(
+          "cherry_pick",
+          async.void(function(f)
+            local selection = nil
 
-          if vim.api.nvim_get_mode().mode == "V" then
-            local status = require("neogit.status")
-            selection = status.get_selected_commits()
-          end
+            if vim.api.nvim_get_mode().mode == "V" then
+              local status = require("neogit.status")
+              selection = status.get_selected_commits()
+            end
 
-          return { commits = selection }
-        end),
+            f { commits = selection }
+          end)
+        ),
         true,
       },
     },
     {
       "StashPopup",
       "Stash",
-      M.open("stash", function()
+      M.open("stash", function(f)
         local status = require("neogit.status")
         local line = status.status_buffer:get_current_line()
-        return {
+        f {
           name = line[1]:match("^(stash@{%d+})"),
         }
       end),
@@ -93,10 +95,10 @@ function M.mappings_table()
     {
       "RevertPopup",
       "Revert",
-      M.open("revert", function()
+      M.open("revert", function(f)
         local status = require("neogit.status")
         local line = status.status_buffer:get_current_line()
-        return { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
+        f { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
       end),
     },
     { "BranchPopup", "Branch", M.open("branch") },
@@ -107,8 +109,8 @@ function M.mappings_table()
 end
 
 function M.test()
-  M.open("echo", function()
-    return "a", "b"
+  M.open("echo", function(f)
+    f("a", "b")
   end)()
 end
 

From 9ec9c91b564deada065cc37b30d3b4aa5db97beb Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sun, 16 Jul 2023 10:32:05 +0200
Subject: [PATCH 05/10] fix: ensure help ordering is the same as before

---
 lua/neogit/popups/init.lua | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index 2af65f071..686dacf67 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -81,6 +81,19 @@ function M.mappings_table()
         true,
       },
     },
+    { "BranchPopup", "Branch", M.open("branch") },
+    { "FetchPopup", "Fetch", M.open("fetch") },
+    { "ResetPopup", "Reset", M.open("reset") },
+    {
+      "RevertPopup",
+      "Revert",
+      M.open("revert", function(f)
+        local status = require("neogit.status")
+        local line = status.status_buffer:get_current_line()
+        f { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
+      end),
+    },
+    { "RemotePopup", "Remote", M.open("remote") },
     {
       "StashPopup",
       "Stash",
@@ -92,19 +105,6 @@ function M.mappings_table()
         }
       end),
     },
-    {
-      "RevertPopup",
-      "Revert",
-      M.open("revert", function(f)
-        local status = require("neogit.status")
-        local line = status.status_buffer:get_current_line()
-        f { commits = { line[1]:match("^(%x%x%x%x%x%x%x+)") } }
-      end),
-    },
-    { "BranchPopup", "Branch", M.open("branch") },
-    { "FetchPopup", "Fetch", M.open("fetch") },
-    { "RemotePopup", "Remote", M.open("remote") },
-    { "ResetPopup", "Reset", M.open("reset") },
   }
 end
 

From 26526cf7f5017e5c59d7c9b0e41cf18eef5cb8f0 Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sun, 16 Jul 2023 10:33:24 +0200
Subject: [PATCH 06/10] fix: lint

---
 lua/neogit/popups/init.lua | 4 ++--
 lua/neogit/status.lua      | 9 ---------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index 686dacf67..e8c807215 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -11,8 +11,8 @@ function M.open(name, f)
   return function()
     local ok, value = pcall(require, "neogit.popups." .. name)
     if ok then
-      assert(value)
-      assert(value.create)
+      assert(value, "popup is not nil")
+      assert(value.create, "popup has create function")
 
       f(value.create)
     else
diff --git a/lua/neogit/status.lua b/lua/neogit/status.lua
index f5e03145b..1cb327443 100644
--- a/lua/neogit/status.lua
+++ b/lua/neogit/status.lua
@@ -912,15 +912,6 @@ local set_folds = function(to)
   refresh(true, "set_folds")
 end
 
-local function cherry_pick()
-  local selection = nil
-  if vim.api.nvim_get_mode().mode == "V" then
-    selection = get_selected_commits()
-  end
-
-  require("neogit.popups.cherry_pick").create { commits = selection }
-end
-
 --- These needs to be a function to avoid a circular dependency
 --- between this module and the popup modules
 local cmd_func_map = function()

From 6662b04820cfeda537c23625e184b157513be429 Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sun, 16 Jul 2023 10:44:30 +0200
Subject: [PATCH 07/10] chore: remove superfluous stash pass through

---
 lua/neogit/popups/help/init.lua | 2 +-
 lua/neogit/popups/init.lua      | 8 --------
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/lua/neogit/popups/help/init.lua b/lua/neogit/popups/help/init.lua
index 067836cce..a015f0169 100644
--- a/lua/neogit/popups/help/init.lua
+++ b/lua/neogit/popups/help/init.lua
@@ -7,7 +7,7 @@ local M = {}
 function M.create(env)
   local p = popup.builder():name("NeogitHelpPopup"):group_heading("Commands")
 
-  local popups = actions.popups(env)
+  local popups = actions.popups()
   for i, cmd in ipairs(popups) do
     p = p:action(cmd.key, cmd.name, cmd.fn)
 
diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index e8c807215..3977a1936 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -32,15 +32,7 @@ function M.mappings_table()
       "HelpPopup",
       "Help",
       M.open("help", function(f)
-        local status = require("neogit.status")
-        local line = status.status_buffer:get_current_line()
-
         f {
-          get_stash = function()
-            return {
-              name = line[1]:match("^(stash@{%d+})"),
-            }
-          end,
           use_magit_keybindings = config.values.use_magit_keybindings,
         }
       end),

From 4119fb964cb5ea43ad5f7f1d6373d25cad6d0b58 Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sun, 16 Jul 2023 11:00:28 +0200
Subject: [PATCH 08/10] chore: export `get_selected_commits`

---
 lua/neogit/status.lua | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lua/neogit/status.lua b/lua/neogit/status.lua
index 1cb327443..6350e5332 100644
--- a/lua/neogit/status.lua
+++ b/lua/neogit/status.lua
@@ -598,7 +598,7 @@ end
 
 --- Returns commits in selection
 ---@return table
-local function get_selected_commits()
+function M.get_selected_commits()
   local first_line = vim.fn.getpos("v")[2]
   local last_line = vim.fn.getpos(".")[2]
 
@@ -611,8 +611,6 @@ local function get_selected_commits()
   return items
 end
 
-M.get_selectet_commits = get_selected_commits
-
 ---Determines if selection contains multiple files
 ---@return boolean
 local function selection_spans_multiple_items_within_section()
@@ -675,7 +673,7 @@ end
 
 local stage_selection = function()
   if selection_spans_multiple_items_within_section() then
-    git.status.stage(unpack(map(get_selected_commits(), function(item)
+    git.status.stage(unpack(map(M.get_selected_commits(), function(item)
       return item.name
     end)))
   else
@@ -689,7 +687,7 @@ end
 
 local unstage_selection = function()
   if selection_spans_multiple_items_within_section() then
-    git.status.unstage(unpack(map(get_selected_commits(), function(item)
+    git.status.unstage(unpack(map(M.get_selected_commits(), function(item)
       return item.name
     end)))
   else
@@ -863,7 +861,7 @@ local discard = function()
   -- These all need to be captured _before_ the get_confirmation() call, since that
   -- seems to effect how vim determines what's selected
   local multi_file = selection_spans_multiple_items_within_section()
-  local files = get_selected_commits()
+  local files = M.get_selected_commits()
 
   local selection = { get_selection() }
 

From 0d673ba07cf56d4c75cf5da77b75eaff5d31a6ff Mon Sep 17 00:00:00 2001
From: Tei Roberts <ten3roberts@gmail.com>
Date: Sun, 16 Jul 2023 11:23:42 +0200
Subject: [PATCH 09/10] fix: argument name

---
 lua/neogit/popups/init.lua | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index 3977a1936..44d18dab6 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -4,8 +4,8 @@ local M = {}
 ---@param f nil|fun(create: fun(...)): any
 --- Creates a curried function which will open the popup with the given name when called
 function M.open(name, f)
-  f = f or function(f)
-    f()
+  f = f or function(c)
+    c()
   end
 
   return function()

From d0e15b515821af27161b14fc1f2bee671b2ef4c7 Mon Sep 17 00:00:00 2001
From: Tei Leelo Roberts <ten3roberts@gmail.com>
Date: Tue, 18 Jul 2023 13:50:17 +0200
Subject: [PATCH 10/10] fix: popup error message

---
 lua/neogit/popups/echo/actions.lua |  2 ++
 lua/neogit/popups/echo/init.lua    |  2 ++
 lua/neogit/popups/init.lua         | 23 ++++++++++-------------
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lua/neogit/popups/echo/actions.lua b/lua/neogit/popups/echo/actions.lua
index 7ccf98ba9..6379c480f 100644
--- a/lua/neogit/popups/echo/actions.lua
+++ b/lua/neogit/popups/echo/actions.lua
@@ -1,3 +1,5 @@
+--- This popup is for unit testing purposes and is not associated to any git command.
+
 local M = {}
 function M.echo(value)
   local notification = require("neogit.lib.notification")
diff --git a/lua/neogit/popups/echo/init.lua b/lua/neogit/popups/echo/init.lua
index 39f29536e..8ebce3f7d 100644
--- a/lua/neogit/popups/echo/init.lua
+++ b/lua/neogit/popups/echo/init.lua
@@ -1,3 +1,5 @@
+--- This popup is for unit testing purposes and is not associated to any git command.
+
 local popup = require("neogit.lib.popup")
 local actions = require("neogit.popups.echo.actions")
 
diff --git a/lua/neogit/popups/init.lua b/lua/neogit/popups/init.lua
index 44d18dab6..dee148c09 100644
--- a/lua/neogit/popups/init.lua
+++ b/lua/neogit/popups/init.lua
@@ -17,7 +17,7 @@ function M.open(name, f)
       f(value.create)
     else
       local notification = require("neogit.lib.notification")
-      notification.create(string.format("No such popup: %q", name), vim.log.levels.ERROR)
+      notification.create(string.format("Failed to load popup: %q\n%s", name, value), vim.log.levels.ERROR)
     end
   end
 end
@@ -26,7 +26,7 @@ end
 ---@return table<string, Mapping>
 function M.mappings_table()
   local config = require("neogit.config")
-  local async = require("plenary.async")
+
   return {
     {
       "HelpPopup",
@@ -57,19 +57,16 @@ function M.mappings_table()
       "Cherry Pick",
       {
         "nv",
-        M.open(
-          "cherry_pick",
-          async.void(function(f)
-            local selection = nil
+        M.open("cherry_pick", function(f)
+          local selection = nil
 
-            if vim.api.nvim_get_mode().mode == "V" then
-              local status = require("neogit.status")
-              selection = status.get_selected_commits()
-            end
+          if vim.api.nvim_get_mode().mode == "V" then
+            local status = require("neogit.status")
+            selection = status.get_selected_commits()
+          end
 
-            f { commits = selection }
-          end)
-        ),
+          f { commits = selection }
+        end),
         true,
       },
     },