feat(ui): keep track of recently used item

[hinell]

https://github.com/mrjones2014/legendary.nvim/assets/8136158/17e19ca5-2bb6-4fe8-8dfb-e8aad186e893

• Remember most recent selection per global list and per itemlist
• Minor refactoring
• For global itemlist, most recently selected item is kept at State.most_recent_item
• For itemgroup-relative selection the State.itemgroup_history[id] map is used

How to Test

• Exposed APIs aren't affected; only UX;

Testing for Regressions

I have tested the following:

• [x] Triggering keymaps from legendary.nvim in all modes (normal, insert, visual)
• [x] Creating keymaps via legendary.nvim, then triggering via the keymap in all modes (normal, insert, visual)
• [x] Triggering commands from legendary.nvim in all modes (normal, insert, visual)
• [x] Creating commands via legendary.nvim, then running the command manually from the command line
• [x] augroup/autocmds created through legendary.nvim work correctly

[mrjones2014]

Thanks for the PR! Neat idea. My initial thought is that ItemGroups are themselves items as well, so we probably don't need a new state, we can probably just update the existing one to be a list instead of a single value. I'll keep looking at this and make some suggestions.

[hinell]

so we probably don't need a new state, we can probably just update the existing one to be a list instead of a single value.

Before this PR, this mechanism was flawed: legendary didn't remember selection inside groups, which was really annoying...

[mrjones2014]

Yeah, I took a look and the existing piece of state is also used for repeating last execution, so we'll need to keep that separate. Will give it a code review this morning.

[mrjones2014]

For docs changes this is all we should need:

diff --git a/README.md b/README.md
index 5bb6537..5eea0db 100644
--- a/README.md
+++ b/README.md
@@ -385,7 +385,9 @@ require('legendary').setup({
   --  }
   -- })
   sort = {
-    -- sort most recently used item to the top
+    -- sort most recently used item to the top,
+    -- this works for both the top-level item list
+    -- as well as items inside of item groups
     most_recent_first = true,
     -- sort user-defined items before built-in items
     user_items_first = true,

[mrjones2014]

@hinell I checked out locally and prepared a patch. Here's the remaining changes we need to make:

diff --git a/lua/legendary/api/executor.lua b/lua/legendary/api/executor.lua
index ee7c175..f164e11 100644
--- a/lua/legendary/api/executor.lua
+++ b/lua/legendary/api/executor.lua
@@ -1,6 +1,7 @@
 local Toolbox = require('legendary.toolbox')
 local Log = require('legendary.log')
 local Config = require('legendary.config')
+local State = require('legendary.data.state')
 local util = require('legendary.util')

 local function update_item_frecency_score(item)
@@ -83,6 +84,7 @@ end
 function M.exec_item(item, context)
   vim.schedule(function()
     M.restore_context(context, function()
+      State.last_executed_item = item
       update_item_frecency_score(item)
       if Toolbox.is_function(item) then
         item.implementation()
@@ -123,11 +125,11 @@ end
 ---@param ignore_filters boolean|nil whether to ignore the filters used when selecting the item, default false
 function M.repeat_previous(ignore_filters)
   local State = require('legendary.data.state')
-  if State.most_recent_item then
+  if State.last_executed_item then
     if not ignore_filters and State.most_recent_filters then
       for _, filter in ipairs(State.most_recent_filters) do
         -- if any filter does not match, abort executions
-        local err, matches = pcall(filter, State.most_recent_item)
+        local err, matches = pcall(filter, State.last_executed_item)
         if not err and not matches then
           Log.warn(
             'Previously executed item no longer matches previously used filters, use `:LegendaryRepeat!`'
@@ -138,7 +140,7 @@ function M.repeat_previous(ignore_filters)
       end
     end
     local context = M.build_context()
-    M.exec_item(State.most_recent_item, context)
+    M.exec_item(State.last_executed_item, context)
   end
 end

diff --git a/lua/legendary/data/itemlist.lua b/lua/legendary/data/itemlist.lua
index a81bfa7..5d5c51e 100644
--- a/lua/legendary/data/itemlist.lua
+++ b/lua/legendary/data/itemlist.lua
@@ -14,6 +14,8 @@ local Log = require('legendary.log')
 ---@field private sorted boolean
 local ItemList = class('ItemList')

+ItemList.TOPLEVEL_LIST_ID = 'toplevel'
+
 ---@private
 function ItemList:initialize()
   self.items = {}
@@ -124,7 +126,13 @@ function ItemList:sort_inplace(opts)

   -- if no items have been added, and the most recent item has not changed,
   -- we're already sorted
-  if self.sorted and (not opts.most_recent_first or (self.items[1] == State.most_recent_item)) then
+  if
+    self.sorted
+    and (
+      not opts.most_recent_first
+      or (self.items[1] == State.itemgroup_history[opts.itemgroup or ItemList.TOPLEVEL_LIST_ID])
+    )
+  then
     return
   end

@@ -188,10 +196,8 @@ function ItemList:sort_inplace(opts)
     end

     if opts.most_recent_first then
-      if opts.itemgroup then
-        return item1 == State.itemgroup_history[opts.itemgroup]
-      else
-        return item1 == State.most_recent_item
+      if item1 == State.itemgroup_history[opts.itemgroup or ItemList.TOPLEVEL_LIST_ID] then
+        return true
       end
     end

@@ -224,9 +230,11 @@ function ItemList:sort_inplace(opts)
   -- sort by most recent last, and after other sorts are done
   -- if most recent is already at top, nothing to do, and attempting to sort will cause
   -- an error since it doesn't need to be sorted
-  if opts.most_recent_first and State.most_recent_item and State.most_recent_item ~= self.items[1] then
+  if
+    opts.most_recent_first and State.itemgroup_history[opts.itemgroup or ItemList.TOPLEVEL_LIST_ID] ~= self.items[1]
+  then
     items = Sorter.mergesort(items, function(item)
-      return item == State.most_recent_item
+      return item == State.itemgroup_history[opts.itemgroup or ItemList.TOPLEVEL_LIST_ID]
     end)
   end

diff --git a/lua/legendary/data/state.lua b/lua/legendary/data/state.lua
index bbbc78a..40249ac 100644
--- a/lua/legendary/data/state.lua
+++ b/lua/legendary/data/state.lua
@@ -2,13 +2,13 @@ local ItemList = require('legendary.data.itemlist')

 ---@class LegendaryState
 ---@field items ItemList
----@field most_recent_item LegendaryItem|nil
+---@field last_executed_item LegendaryItem|nil
 ---@field most_recent_filters LegendaryItemFilter[]|nil
----@field itemgroup_history ItemList[]
+---@field itemgroup_history table<string, LegendaryItem>
 local M = {}

 M.items = ItemList:create()
-M.most_recent_item = nil
+M.last_executed_item = nil
 M.most_recent_filters = nil
 M.itemgroup_history = {}

diff --git a/lua/legendary/ui/init.lua b/lua/legendary/ui/init.lua
index d33b0ab..17aa10e 100644
--- a/lua/legendary/ui/init.lua
+++ b/lua/legendary/ui/init.lua
@@ -6,12 +6,13 @@ local Toolbox = require('legendary.toolbox')
 local Format = require('legendary.ui.format')
 local Executor = require('legendary.api.executor')
 local Log = require('legendary.log')
+local ItemList = require('legendary.data.itemlist')

 ---@class LegendaryUi
 ---@field select fun(opts:LegendaryFindOpts)
 local M = {}

----@class LegendaryFindOpts
+---@class LegendaryFindOpts : ItemListSortInplaceOpts
 ---@field itemgroup string Find items in this item group only
 ---@field filters LegendaryItemFilter[]
 ---@field select_prompt string|fun():string
@@ -82,11 +83,7 @@ local function select_inner(opts, context, itemlist)
       return
     end

-    if opts.itemgroup then
-      State.itemgroup_history[opts.itemgroup] = selected
-    else
-      State.most_recent_item = selected
-    end
+    State.itemgroup_history[opts.itemgroup or ItemList.TOPLEVEL_LIST_ID] = selected

     if Toolbox.is_itemgroup(selected) then
       local item_group_id = selected:id()

[mrjones2014]

If you apply the patch, I'll be happy to test and merge!

[mrjones2014]

@hinell one small issue from CI, then I can merge:

Checking lua/legendary/api/executor.lua           1 warning

    lua/legendary/api/executor.lua:127:9: shadowing upvalue State on line 4

[dundargoc]

I dunno what legendary.nvim is, but this is clearly critical. Smashed that approve button!

[mrjones2014]

I dunno what legendary.nvim is

lol you approved the PR two hours ago though 😆