Skip to content

fix: improve set_cursor_position with pcall for fault tolerance#14

Open
mskadu wants to merge 9 commits into
vladdoster:masterfrom
mskadu:fix/improve-api-call-fault-tolerance
Open

fix: improve set_cursor_position with pcall for fault tolerance#14
mskadu wants to merge 9 commits into
vladdoster:masterfrom
mskadu:fix/improve-api-call-fault-tolerance

Conversation

@mskadu

@mskadu mskadu commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wrap set_cursor_position in pcall to catch errors gracefully
  • Add error notification via vim.notify when Neovim API calls fail
  • Add tests to verify fault tolerance behavior

Changes

This prevents the plugin from crashing when unexpected API errors occur.

@vladdoster

vladdoster commented May 28, 2026

Copy link
Copy Markdown
Owner

@mskadu,

Why did you delete all the comments?

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

📊 Test Coverage Report

Overall Coverage: 97.78%

📋 Detailed Coverage Report
==============================================================================
lua/remember.lua
==============================================================================
    --
    -- Author: vladdoster <mvdoster@gmail.com>
    -- Version: 1.5.1
    --
    -- Based on https://github.com/farmergreg/vim-lastplace/
    --
    -- This work is licensed under the terms of the MIT license.
    -- For a copy, see <https://opensource.org/licenses/MIT>.
    --

 18 local g = vim.g
 18 local bo = vim.bo
 18 local fn = vim.fn
 18 local api = vim.api
 18 local cmd = vim.cmd

 18 local M = {}

 18 local config = {
 18   ignore_filetype = { "gitcommit", "gitrebase", "svn", "hgcommit", "dap-repl" },
 18   ignore_buftype = { "quickfix", "nofile", "help" },
 18   open_folds = true,
 18   dont_center = false,
    }

 18 function M.setup(options)
  9   if options["ignore_filetype"] then
  2     config["ignore_filetype"] = options["ignore_filetype"]
      end

  9   if options["ignore_buftype"] then
  2     config["ignore_buftype"] = options["ignore_buftype"]
      end

  9   if options["open_folds"] then
  1     config["open_folds"] = options["open_folds"]
      end

  9   if options["dont_center"] then
  3     config["dont_center"] = options["dont_center"]
      end
    end

 18 function set_cursor_position()
      -- Return if we have a buffer or filetype we want to ignore
 77   for _, k in pairs(config["ignore_buftype"]) do
 59     if bo.buftype == k then
  4       return
        end
      end

 84   for _, k in pairs(config["ignore_filetype"]) do
 72     if bo.filetype == k then
  6       return
        end
      end

      -- Return if the file doesn't exist, like a new and unsaved file
 12   if fn.empty(fn.glob(fn.expand("%"))) ~= 0 then
  1     return
      end

 11   local cursor_position = api.nvim_buf_get_mark(0, '"')
 11   local row = cursor_position[1]
 11   local col = cursor_position[2]

      -- If the saved row is less than the number of rows in the buffer,
      -- then continue
 11   if row > 0 and row <= api.nvim_buf_line_count(0) then
        -- If the last row is visible within this window, like in a very short
        -- file, or user requested us not centering the screen, just set the cursor
        -- position to the saved position
  9     if api.nvim_buf_line_count(0) == fn.line("w$") or config["dont_center"] then
  5       api.nvim_win_set_cursor(0, cursor_position)

          -- If we're in the middle of the file, set the cursor position and center
          -- the screen
  4     elseif api.nvim_buf_line_count(0) - row > ((fn.line("w$") - fn.line("w0")) / 2) - 1 then
  3       api.nvim_win_set_cursor(0, cursor_position)
  3       cmd("norm! zz")

          -- If we're at the end of the screen, set the cursor position and move
          -- the window up by one with C-e. This is to show that we are at the end
          -- of the file. If we did "zz" half the screen would be blank.
        else
  1       api.nvim_win_set_cursor(0, cursor_position)
  1       api.nvim_feedkeys(api.nvim_replace_termcodes("<c-e>", true, false, true), "n", false)
        end
      end

      -- If the row is within a fold, make the row visible and recenter the screen
 11   if api.nvim_eval("foldclosed('.')") ~= -1 and config["open_folds"] then
  2     cmd("norm! zvzz")
      end
    end

 36 api.nvim_create_autocmd({ "BufWinEnter" }, {
      callback = function()
**0     set_cursor_position()
      end,
    })

 18 return M

==============================================================================
Summary
==============================================================================

File             Hits Missed Coverage
-------------------------------------
lua/remember.lua 44   1      97.78%
-------------------------------------
Total            44   1      97.78%


Coverage report generated by luacov

@mskadu

mskadu commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

@mskadu,

Why did you delete all the comments?

Oops — the comments were dropped unintentionally during the pcall restructure. The function body re-indentation caused them to be lost.

I've restored them all in the latest commit (but please do let me know if I've missed any). While I was at it, blank-line separators between top-level sections are back too. Clearly I need a better code reformatter (or config for it) :)

Restore original field ordering (ignore_filetype before ignore_buftype)
in config table and setup function. Restore blank line after license
block and trailing newline. These were pure formatting noise unrelated
to the pcall fix.
@mskadu

mskadu commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Cleaned up the diff — removed unnecessary config field reordering, blank line changes, and trailing newline changes. The diff now only contains the pcall wrapping and error handler. All 18 tests still pass.

Ready for re-review whenever you have a moment. Thanks!

@mskadu

mskadu commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

👋 hola! The diff was cleaned up after your last round of feedback. Should be good for another look whenever you have time. Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants