Skip to content

bug(image): Image:del() sends the loop index as the placement id, so p never matches #2946

Description

@kalinichenko88

Did you check docs and existing issues?

  • I have read all the snacks.nvim docs
  • I have updated the plugin to the latest version before submitting this issue
  • I have searched the existing issues of snacks.nvim
  • I have searched the existing issues of plugins related to this issue

Neovim version (nvim -v)

NVIM v0.12.5

Operating system/version

macOS 26.6

Describe the bug

Image:del() deletes a placement with the index of the loop, not the
placement id, so the escape sequence it writes names a placement the terminal
does not have. Line numbers are from main today (v2.31.0).

lua/snacks/image/image.lua:199:

function M:del(pid)
  for id, p in ipairs(pid and { pid } or vim.tbl_keys(self.placements)) do
    if self.placements[p] then
      terminal.request({ a = "d", d = "i", i = self.id, p = id })  -- p = the ipairs index
      self.placements[p] = nil                                     -- p = the real id
    end
  end

vim.tbl_keys returns the placement ids as a list, so ipairs yields
id = 1, 2, 3 … and p = the real id. The table bookkeeping on the next line
uses p and is correct; only the request is wrong.

Two things make the mismatch total rather than occasional:

  1. _pid starts at 10 (image.lua:17, incremented at :188), so placement
    ids begin at 11. An index of 1, 2, 3 … cannot equal them.
  2. The only caller passes the id explicitly — placement.lua:195,
    self.img:del(self.id) — and ipairs({ pid }) yields exactly one pair with
    id = 1. That path always sends p = 1, whatever the placement is.

Meanwhile the placement is created with the right id: p = self.id at
placement.lua:434 and :556. So snacks places p=11 and deletes p=1.

In today's nvim --headless --clean:

placement 7 -> request sends p=1
placement 12 -> request sends p=2
placement 3 -> request sends p=3
explicit del(7) -> request sends p=1

The visible damage is bounded by the fallback right below the loop: when the
image's last placement goes, { a = "d", d = "i", i = self.id } is sent
with no p and does clear it. So a one-placement image is cleaned up by
accident, and the bug shows on an image that has more than one placement — the
same file open in two windows or two buffers. Closing one leaves its picture on
screen, because the request that was supposed to remove it named p=1.

Steps To Reproduce

  1. Open an image file in a terminal that speaks the kitty graphics protocol.
  2. :vsplit and open the same image in the second window, so one
    snacks.Image carries two placements.
  3. Close one of the windows.
  4. The picture belonging to the closed placement stays on screen. Snacks'
    own table has already dropped it, so nothing redraws over it.

Expected Behavior

The delete names the placement being deleted:

terminal.request({ a = "d", d = "i", i = self.id, p = p })

Repro

The defect is in the index, so it reproduces without a terminal:

-- nvim --headless --clean -c 'luafile repro.lua' -c q
local placements = { [11] = "first", [12] = "second" } -- ids as _pid hands them out
for id, p in ipairs(vim.tbl_keys(placements)) do
  print(("placement %d -> del() sends p=%d"):format(p, id))
end
for id, p in ipairs({ 12 }) do -- the placement.lua:195 path
  print(("del(%d) -> sends p=%d"):format(p, id))
end

One note on what I did not verify: I have not watched the stale picture in a
terminal myself. I am on WezTerm, where
wezterm#2422 leaves deleted
placements on screen for its own reasons, so a visual test there cannot tell the
two apart. Everything above is from the code and the printed request values.

Activity

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

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