Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The harpoon list can have the wrong length #555

Open
abeldekat opened this issue Apr 5, 2024 · 4 comments
Open

The harpoon list can have the wrong length #555

abeldekat opened this issue Apr 5, 2024 · 4 comments
Labels
bug Something isn't working harpoon2

Comments

@abeldekat
Copy link

abeldekat commented Apr 5, 2024

wrong_length.mp4

Hello @ThePrimeagen,

First off, thanks for merging this PR

Using my plugin harpoonline, I discovered that the length of the list can be wrong. I use:

require("harpoon"):list():length()

Up until recently this was working just fine.

See the video.

Reproduce:

  1. Start from scratch
  2. Harpoon 2 files
  3. Open the UI
  4. Delete both files
  5. Reopen the UI: No items
  6. Notice that harpoonline still shows 1: -> The latest call to length returned 1. See this code fragment
  7. Use vim.print and notice that the length of the list is indeed 1 instead of 0
@ThePrimeagen
Copy link
Owner

Should be easy fix

@ThePrimeagen ThePrimeagen added bug Something isn't working harpoon2 labels Apr 6, 2024
@abeldekat
Copy link
Author

I included a test for this scenario in the Harpoonline plugin.

@abeldekat
Copy link
Author

When implementing toggle functionality in my config I found another side effect of this issue.

  1. Open a file
  2. require("harpoon"):list:add()
  3. Open another file
  4. require("harpoon"):list:add()
  5. Go to the first file
  6. require("harpoon"):list:remove_at(1)

Open the UI: There are two items. The first item is empty.

I would expect one item, and the second file on index 1.

I think this part might be the problem.

@abeldekat
Copy link
Author

abeldekat commented Apr 30, 2024

The toggle is working when using replace_at:

A.toggle = function()
  local list = H.get_current_list()
  local active_idx = H.find_active_idx(list)
  local len = #list.items
  if not active_idx or len == 0 then
    list:add()
  elseif active_idx == len then -- last item
    list:remove_at(active_idx) --
  else
    -- must replace with the last item in order to avoid gaps
    list:replace_at(active_idx, list.items[len])
  end
end

Unfortunately, I cannot submit a PR because I do not understand the purpose of methods determine_length and guess_length. As list.items is if of type HarpoonItem[], why not use table.insert and table.remove?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working harpoon2
Projects
None yet
Development

No branches or pull requests

2 participants