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

Harpoon doesn't save cursor position after quit #441

Open
romado77 opened this issue Dec 22, 2023 · 23 comments · May be fixed by #533
Open

Harpoon doesn't save cursor position after quit #441

romado77 opened this issue Dec 22, 2023 · 23 comments · May be fixed by #533

Comments

@romado77
Copy link

romado77 commented Dec 22, 2023

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" })
@theKnightsOfRohan
Copy link

theKnightsOfRohan commented Dec 23, 2023

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.

@theKnightsOfRohan
Copy link

theKnightsOfRohan commented Dec 23, 2023

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>

@roycrippen4
Copy link

Any updates or workarounds?

@ThePrimeagen
Copy link
Owner

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

@theKnightsOfRohan
Copy link

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.

@theKnightsOfRohan
Copy link

theKnightsOfRohan commented Dec 28, 2023

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.

@roycrippen4
Copy link

@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?

@arnevm123
Copy link

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

@ThePrimeagen
Copy link
Owner

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

I'll stream it to

@bsolar17
Copy link

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.

@ThePrimeagen
Copy link
Owner

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

This will prevent the aforementioned problem from happening

@maxrzaw
Copy link

maxrzaw commented Dec 31, 2023

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.

@maxrzaw
Copy link

maxrzaw commented Jan 2, 2024

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

@arnevm123
Copy link

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,

@ThePrimeagen
Copy link
Owner

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

@roycrippen4
Copy link

four-to-six-weeks-later

@theKnightsOfRohan
Copy link

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

@roycrippen4
Copy link

@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.

@whoop-t
Copy link

whoop-t commented Feb 25, 2024

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

davvid added a commit to davvid/harpoon that referenced this issue Feb 29, 2024
Vim has a builtin mechanism for saving and restoring cursor positions
and marks across restarts. Leverage mkview and loadview to make harpoon
restore marks and cursor positions after quitting.

https://www.reddit.com/r/vim/comments/ex1vlv/comment/fg63erw

Closes: ThePrimeagen#441
@davvid
Copy link
Contributor

davvid commented Feb 29, 2024

: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.

@GitMurf
Copy link

GitMurf commented Mar 8, 2024

@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.

@davvid
Copy link
Contributor

davvid commented Mar 10, 2024

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.

davvid added a commit to davvid/harpoon that referenced this issue Mar 11, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Mar 11, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
@davvid davvid linked a pull request Mar 11, 2024 that will close this issue
@davvid
Copy link
Contributor

davvid commented Mar 11, 2024

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

davvid added a commit to davvid/harpoon that referenced this issue Mar 11, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Mar 11, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Mar 11, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Mar 11, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Apr 13, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Apr 13, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Apr 13, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Apr 13, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Apr 13, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
davvid added a commit to davvid/harpoon that referenced this issue Apr 13, 2024
Harpoon was not remembering the cursor position after quitting nvim.
Ensure that the cursor information is always saved and applied so that
we get back to exactly where we last were in the harpooned file.

Closes: ThePrimeagen#441
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants