Skip to content

screenpos() throws E966 and crashes rendering when a cached image row goes stale (e.g. after a wholesale buffer rewrite by another plugin) #372

Description

@Y-jiji

Summary

renderer.lua's render() calls vim.fn.screenpos() with an image's cached row without checking that row still exists in the buffer. If a row ends up stale (see "how to reproduce" below), the call throws E966: Invalid line number instead of the graceful "not visible" handling the surrounding code clearly expects.

vim.schedule callback: Vim:E966: Invalid line number: 6
stack traceback:
        [C]: in function 'screenpos'
        .../image.nvim/lua/image/renderer.lua:243: in function 'render'
        .../image.nvim/lua/image/image.lua:82: in function 'render'
        .../image.nvim/lua/image/init.lua:154: in function 'render_window_images'
        .../image.nvim/lua/image/init.lua:177: in function 'callback'
        .../image.nvim/lua/image/utils/render_scheduler.lua:18: in function <...render_scheduler.lua:13>

How to reproduce

  1. Open a markdown file with a real image link (![alt](photo.png)), in a terminal with a working image.nvim backend (kitty, etc.), markdown integration enabled. Let it render.
  2. Run: :lua vim.api.nvim_buf_set_lines(0, 0, -1, false, vim.api.nvim_buf_get_lines(0, 0, -1, false)) — reads the whole buffer back and rewrites it completely unchanged, as one wholesale replace instead of a no-op.
  3. Scroll or resize the window (or just wait for the next redraw) to trigger a re-render.
  4. Check :messages for E966: Invalid line number, sourced from renderer.lua.

Suggested fix

Provisional Patch Bounds-check the row against the buffer's current line count before calling screenpos(), and treat "row doesn't exist" the same as the existing "off viewport" handling right below it, instead of letting the error surface:

local bufnr = vim.api.nvim_win_get_buf(image.window)
if original_y + 1 > vim.api.nvim_buf_line_count(bufnr) then
  -- same handling as the "below viewport" branch just below this call
  if state.images[image.id] and state.images[image.id] ~= image then state.images[image.id]:clear(true) end
  state.images[image.id] = image
  return false
end

(or wrap the screenpos call itself in pcall as defense in depth, in case there are other paths that hit the same error.)

The actual bug might be something deeper. Neovim pushes extmark to the end of replaced range. The provisional fix only makes sure that the plugin does not crash. But other edits (instead of full range replacement, do n-1 line replacement) might make the image render at a wrong line number. To fix this in a robust way, we may have to change how this plugin works entirely.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions