Fix window picker's hint logic to filter to only valid, visible windo…

…ws (#71)

* Fix window picker's hint logic to filter to only valid, visible windows

* Update test matrix with new neovim instances

* Update utils.org_file() to actually write to the file, and adjust tests to use orgfile.metadata.mtime instead of 0

* Disable nightly from github tests until neovim/neovim#30792 is fixed

.github/workflows/tests.yml

@@ -36,11 +36,13 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        version: [v0.9.2, v0.9.4, v0.9.5, nightly]
+        # NOTE: Nightly disabled until neovim/neovim#30792 is fixed!
+        #       Once it is fixed, add "nightly" back to version list.
+        version: [v0.9.4, v0.9.5, v0.10.2, v0.10.3]
         os: [ubuntu-latest]
         include:
           - os: windows-latest
-            version: v0.9.5
+            version: v0.10.3
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4

lua/org-roam/core/ui/window-picker/hint.lua

@@ -43,6 +43,7 @@ function M:new(opts)
     return instance
 end
 
+---@private
 ---@param window integer
 ---@return {x:number, y:number}
 function M:__get_float_win_pos(window)
@@ -57,6 +58,7 @@ function M:__get_float_win_pos(window)
     return point
 end
 
+---@private
 ---@param lines string[]
 ---@return string[]
 function M.__add_big_char_margin(lines)
@@ -87,6 +89,7 @@ function M.__add_big_char_margin(lines)
     return centered_lines
 end
 
+---@private
 ---@param window integer
 ---@param char string
 ---@return integer
@@ -119,16 +122,37 @@ function M:__show_letter_in_window(window, char)
     return window_id
 end
 
+---Draws a letter as a window on top of each valid, visible window supplied.
 ---@param windows integer[]
 function M:draw(windows)
-    for i, window in ipairs(windows) do
+    -- Filter out to only include valid windows
+    local valid_windows = {}
+    for _, window in ipairs(windows) do
+        if self:__should_draw_on_window(window) then
+            table.insert(valid_windows, window)
+        end
+    end
+
+    -- If we still have too many windows to populate with our characters,
+    -- fail cleanly with a recommendation
+    local max_windows = self:__max_windows()
+    assert(
+        #valid_windows <= max_windows,
+        table.concat({
+            "Too many valid, visible windows!",
+            "Increase the characters available to the window picker.",
+        }, " ")
+    )
+
+    for i, window in ipairs(valid_windows) do
         local char = string.sub(self.config.chars, i, i)
         local big_char = assert(font[char:lower()], "font missing for " .. char:lower())
         local window_id = self:__show_letter_in_window(window, big_char)
         table.insert(self.__windows, window_id)
     end
 end
 
+---Clears the hint.
 function M:clear()
     for _, window in ipairs(self.__windows) do
         if vim.api.nvim_win_is_valid(window) then
@@ -141,4 +165,25 @@ function M:clear()
     self.__windows = {}
 end
 
+---@private
+---Returns true if the window should be drawn on, meaning it's valid and visible.
+---@param window integer #handle of the window
+---@return boolean
+function M:__should_draw_on_window(window)
+    if not vim.api.nvim_win_is_valid(window) then
+        return false
+    end
+
+    ---@type vim.api.keyset.win_config
+    local config = vim.api.nvim_win_get_config(window)
+    return config.relative == ""
+end
+
+---@private
+---Returns the maximum number of valid, visible windows supported.
+---@return integer
+function M:__max_windows()
+    return string.len(self.config.chars)
+end
+
 return M

spec/core_file_spec.lua

@@ -60,7 +60,7 @@ describe("org-roam.core.file", function()
                         { column = 4, offset = 143, row = 6 },
                     },
                 },
-                mtime = 0,
+                mtime = orgfile.metadata.mtime,
                 range = {
                     end_ = {
                         column = MAX_NUMBER,
@@ -146,7 +146,7 @@ describe("org-roam.core.file", function()
                         { column = 8, offset = 323, row = 12 },
                     },
                 },
-                mtime = 0,
+                mtime = orgfile.metadata.mtime,
                 range = {
                     end_ = {
                         column = 0,
@@ -299,7 +299,7 @@ describe("org-roam.core.file", function()
                         },
                     },
                 },
-                mtime = 0,
+                mtime = orgfile.metadata.mtime,
                 range = {
                     end_ = {
                         column = MAX_NUMBER,
@@ -329,7 +329,7 @@ describe("org-roam.core.file", function()
                         },
                     },
                 },
-                mtime = 0,
+                mtime = orgfile.metadata.mtime,
                 range = {
                     end_ = {
                         column = 0,
@@ -359,7 +359,7 @@ describe("org-roam.core.file", function()
                         },
                     },
                 },
-                mtime = 0,
+                mtime = orgfile.metadata.mtime,
                 range = {
                     end_ = {
                         column = 0,

spec/utils.lua

@@ -111,7 +111,7 @@ end
 
 ---Creates a new orgfile, stripping common indentation.
 ---@param content string
----@param opts? {path?:string}
+---@param opts? {path?:string, skip_write?:boolean}
 ---@return OrgFile
 function M.org_file(content, opts)
     opts = opts or {}
@@ -121,6 +121,12 @@ function M.org_file(content, opts)
         filename = filename .. ".org"
     end
 
+    -- Write to the file to ensure that it matches the content
+    -- to avoid testing race conditions somehow
+    if not opts.skip_write then
+        M.write_to(filename, content)
+    end
+
     ---@type OrgFile
     local file = OrgFile:new({
         filename = filename,