[Harpoon2] Calling `remove_at` to manipulate elements when using Telescope UI causes items list to become malformed

[PedroBinotto]

• May be fixed by: #628;

---

• Possibly related:
  • #477;
  • #499;
  • #573;

---

• What issue are you having that you need harpoon to solve?

When using Harpoon via the Telescope UI, I would like to be able to remove items from the Harpoon list; currently, there are some ways to do it, such as extracting the index in selected_entry.index from telescope.actions.state, and using that to index the item to be removed from the list:

-- excerpt from my neovim config

-- alias definitions:
local harpoon = require("user.harpoon")
local telescope_conf = require("telescope.config").values
local telescope_state = require("telescope.actions.state")
local telescope_finders = require("telescope.finders")
local telescope_pickers = require("telescope.pickers")

-- [...]

function M.toggle_telescope(harpoon_files)
	local file_paths = harpoon_get_paths(harpoon_files)

	telescope_pickers
		.new({}, {
			prompt_title = "Harpoon",
			finder = harpoon_make_finder(file_paths),
			previewer = telescope_conf.file_previewer({}),
			sorter = telescope_conf.generic_sorter({}),
			attach_mappings = function(prompt_buffer_number, map)
				map("i", "<M-d>", function()
				
				        -- HERE
					local selected_entry = telescope_state.get_selected_entry()
					local current_picker = telescope_state.get_current_picker(prompt_buffer_number)
					harpoon:list():remove_at(selected_entry.index)
					current_picker:refresh(harpoon_make_finder(harpoon_get_paths(harpoon:list())))
				        -- HERE

				end)

				return true
			end,
		})
		:find()
end

-- aux functions:

local harpoon_get_paths = function(files)
	local paths = {}
	for _, item in ipairs(files.items) do
		table.insert(paths, item.value)
	end
	return paths
end

local function harpoon_make_finder(paths)
	return telescope_finders.new_table({ results = paths })
end

• Why doesn't the current config help?

This method leads to problems, however. As it turns out, calling remove_at and passing it an index will leave a list with a "hole" where the removed item once was, and every item that comes after will be explicitly indexed, like so:

{  -- BEFORE
  {
    context = {
      col = 0,
      row = 1
    },
    value = "file_1"
  }, {
    context = {
      col = 0,
      row = 1
    },
    value = "file_2"
  }, {
    context = {
      col = 1,
      row = 2
    },
    value = "file_3"
  }, {
    context = {
      col = 0,
      row = 1
    },
    value = "file_4"
  }, {
    context = {
      col = 0,
      row = 1
    },
    value = "file_5"
  }
}

-- harpoon:list():remove_at(3)

{  -- AFTER
  {
    context = {
      col = 0,
      row = 1
    },
    value = "file_1"
  }, {
    context = {
      col = 0,
      row = 1
    },
    value = "file_2"
  },
  [4] = {
    context = {
      col = 0,
      row = 1
    },
    value = "file_4"
  },
  [5] = {
    context = {
      col = 0,
      row = 1
    },
    value = "file_5"
  }
}

This alone wouldn't be a problem, but whenever an operation requires looping over the list, the iterator will halt once it reaches the "missing" element. That includes the Telecope UI rendering, which would only display elements up to that position, and, for whatever reason, this also seems to impact the "persistence" of the harpooned items on exit; harpoon:list().items only contains items up to that index once you restart vim (possibly caused by the use of ipairs() over self.items in HarpoonList:encode, which halts once it reaches the "hole"); this means that the items after the removed element are LOST once you exit vim:

{
  {
    context = {
      col = 0,
      row = 1
    },
    value = "file_1"
  }, {
    context = {
      col = 0,
      row = 1
    },
    value = "file_2"
  }
}

There is a possible workaround if you want to make it work nonetheless, as suggested by @Ocholla-T in this comment (#499); it works by manually removing the item from the table, instead of making the acutal API call:

function M.toggle_telescope(harpoon_files)
	local file_paths = harpoon_get_paths(harpoon_files)

	telescope_pickers
		.new({}, {
			prompt_title = "Harpoon",
			finder = harpoon_make_finder(file_paths),
			previewer = telescope_conf.file_previewer({}),
			sorter = telescope_conf.generic_sorter({}),
			attach_mappings = function(prompt_buffer_number, map)
				map("i", "<M-d>", function()
					local selected_entry = telescope_state.get_selected_entry()
					local current_picker = telescope_state.get_current_picker(prompt_buffer_number)
-                                       harpoon:list():remove_at(selected_entry.index)
+					table.remove(harpoon:list().items, selected_entry.index)
					current_picker:refresh(harpoon_make_finder(harpoon_get_paths(harpoon:list())))
				end)

				return true
			end,
		})
		:find()
end

• What proposed API changes are you suggesting?

This works! But it seems as if there's a bug in the actual list implementation, though it isn't caught by the unit tests nor seems to cause any other apparent problems.

I'm still doing some debugging over this issue, but I was able to roughly identify the point where the "bug" occurs:

---@return HarpoonList
function HarpoonList:remove_at(index)
    if self.items[index] then
        Logger:log(
            "HarpoonList:removeAt",
            { item = self.items[index], index = index }
        )
        self.items[index] = nil
---     ^^^^^^^^^^^^^^^^^^^^^^^ from this point onwards, the full items list is no longer functional
        if index == self._length then
            self._length = determine_length(self.items, self._length)
        end
        Extensions.extensions:emit(
            Extensions.event_names.REMOVE,
            { list = self, item = self.items[index], idx = index }
        )
    end
    return self
end

from: harpoon/lua/harpoon/list.lua#L205