ThePrimeagen/harpoon

Harpoon doesn't save cursor position after quit

Opened this issue ยท 23 comments

I've noticed an issue with Harpoon related to the persistence of cursor positions. When a file is harpooned, the cursor position is saved upon quitting neovim (I could see this from harpoon.json). However, when reopening the harpooned file and moving the cursor to a new position, the Harpoon doesn't preserve the updated cursor position if I quit from neovim again. So every time I open harpooned file it jumps to a position which was initially saved when file was harpooned.

My configuration:

local harpoon = require("harpoon")

harpoon:setup({ settings = {
	save_on_toggle = true,
	sync_on_ui_close = true,
} })


vim.keymap.set("n", "<leader>a", function()
	harpoon:list():append()
end, { desc = "Add current file to harpoon" })
vim.keymap.set("n", "<leader>h", function()
	harpoon.ui:toggle_quick_menu(harpoon:list())
end, { desc = "Toggle harpoon menu" })

vim.keymap.set("n", "<C-1>", function()
	harpoon:list():select(1)
end, { desc = "Select harpoon item 1" })
vim.keymap.set("n", "<C-2>", function()
	harpoon:list():select(2)
end, { desc = "Select harpoon item 2" })
vim.keymap.set("n", "<C-3>", function()
	harpoon:list():select(3)
end, { desc = "Select harpoon item 3" })
vim.keymap.set("n", "<C-4>", function()
	harpoon:list():select(4)
end, { desc = "Select harpoon item 4" })

I can replicate this issue. Here is a video, in two parts because size requirements:

Vid_P1.mov
Vid_P2.mov

Harpoon isn't updating the global list when you move places. The logger shows the parent list, and not the updated list. The updated list is also not being written. In addition, harpooning at the new position doesn't do anything, and it will still move to the initial harpoon position upon a new session.

It's also a failing test in harpoon_spec.lua, but I have no idea why the error is ATTENTION, or why utils.create_file is bugging out lol

Testing:        /Users/rohanseth/Documents/GitHub/harpoon/lua/harpoon/test/harpoon_spec.lua
Fail    ||      harpoon when we change buffers we update the row and column
            ./lua/harpoon/test/utils.lua:61: Vim:E325: ATTENTION
            
            stack traceback:
                ./lua/harpoon/test/utils.lua:61: in function 'create_file'
                ...cuments/GitHub/harpoon/lua/harpoon/test/harpoon_spec.lua:20: in function <...cuments/GitHub/harpoon/lua/harpoon/test/harpoon_spec.lua:16>

Any updates or workarounds?

I'm going to have to think about a way to be able to debug. It works on my machine, but that sucks. I'll have to make the log be able to save to a file. And then have you share the file

I've isolated it down to the BufLeave function definition in lua/harpoon/config.lua, line 184. The log statement which should be updating position isn't getting called, yet the behaviour is replicated regardless. I've also looked for other places where item.context is updated, but none of those are changed anywhere else, only referenced by the nvim_win_set_cursor function. An explanation I'm thinking of is that this behaviour is being done by neovim inherently, rather than harpoon for whatever reason.

Additionally, after a but more searching, I'm seeing that the BufLeave function defined by the default config doesn't seem to be called anywhere, as there is no place where an autocmd is created using that function.

@ThePrimeagen two questions: What is your config; the default with both sync_on_ui_close and save_on_ui_toggle set to false? And do your marks persist if you close a buffer or you exit neovim?

I have the same issue + I don't get line numbers anymore sometimes, not sure how that happened.

I'll see if I can spend some time on this tonight.

I'll stream it to

I also have this issue and I thought it was due to how BufLeave is supposed to work as the documentation states the following:

Before leaving to another buffer. Also when leaving or closing the current window and the new current window is not for the same buffer.

Not used for ":qa" or ":q" when exiting Vim.

I'm going to rework how the files are stored.

This will prevent the aforementioned problem from happening

I am currently writing a custom list and I think I discovered what is causing this. I think the issue is in BufLeave. It is looking up by the display name and the display name is not always the same as vim.api.nvim_buf_get_name(bufnr).

I added some log statements to BufLeave and it finds the item when the display name is the path from root, but does not find it when it is a relative path.

Having the full file path as part of the context would be helpful for solving this issue and I think it would be helpful for many other use cases.

I have been looking into this a bit and there are at least two issues going on here.

  1. The display name does not match the name of the file so the current BufLeave function does not always update the position. I have recreated this with a unit test and have a fix for this case.
  2. VimLeavePre is called, but in _for_each_list() seen is not true and lists is sometimes nil.

Edit:
I have issue 2 narrowed down to a couple things related to _for_each_list():

  • There is no default VimLeavePre function
  • self.data.seen is nil or empty
  • self.lists is nil or empty

Workaround that seems to work, but could use (a lot of) improvement:

VimLeavePre = function(_, list)
	for bufnr = 1, vim.fn.bufnr("$") do
		if vim.fn.buflisted(bufnr) == 1 then
			local bufname = vim.api.nvim_buf_get_name(bufnr)
			local item = nil
			for _, it in ipairs(list.items) do
				local value = it.value
				if value == bufname then
					item = it
					break
				end
			end
			if item then
				vim.api.nvim_set_current_buf(bufnr)
				local pos = vim.api.nvim_win_get_cursor(0)
				item.context.row = pos[1]
				item.context.col = pos[2]
			end
		end
	end
end,

I'm going to be changing how I save files here soon.

Instead of one big configuration, I will have many small files. This is going to allow me to not wait until exit to save

four-to-six-weeks-later

Unfortunately, Prime is still APM-Brained. We should probably expect harpoon to be slowed down for at least another month, maybe even two.

@theKnightsOfRohan I get it. It's open source, it's his project, and he can do as little or as much as he wants with it.

Still issue at time of this comment, just bumping for others who end up here

:help restore-cursor provides a stock way to save cursor positions for all files, not just for files known to harpoon. There are references online that suggest that vim shipped with a default.vim that handled this in the past.

Another approach is to leverage the viminfo mkview and loadview features to restore marks and cursors after quitting. The basic idea is sketched out here: https://www.reddit.com/r/vim/comments/ex1vlv/comment/fg63erw/

I took that and minimally wired it into harpoon2 in #525 as a proof of concept. The code's not terrible so it could be merged as-is, but it does call into question whether the context table needs to keep track of row and col anymore now that vim's handling that part.

I'm not sure if this is the best way since it's setup to save/restore on switch and sync unconditionally, but it does work pretty reliably.

@davvid I have ran into some weird issues with cwd, folds etc. when using mkview, etc. Depending on your global settings, more stuff than you want could be saved unfavorably and I think the purpose of harpoon is to be pretty explicit and defined about what it is trying to harpoon / focus in on. Just my 2 cents that bringing views in could get messy pretty quickly based on my experience with other plugins.

Thanks @GitMurf +1 to that. Indeed, it'd be better to make it so that the row and col fields are always correct so that a proper and more minimalist approach can be taken.

Another reason why we'd want a more minimalist implementation is that mkview and loadview are slower than setting just setting the cursor directly.

Alrighty @GitMurf take a look at #533 ~ it's behaving perfectly as far as I can tell.