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

Harpoon2: Wrong state is loaded when working with multiple tabs - having different working dirs #612

Open
stroiman opened this issue Jun 25, 2024 · 8 comments

Comments

@stroiman
Copy link

WARNING
If this is about Harpoon1, the issue will be closed. All support and everything of harpoon1 will be frozen on master until 4/20 or 6/9 and then harpoon2 will become master

Please use harpoon2 for branch

When opening a tab, and setting a different working folder in the new tab, the harpoon list is not loaded next time I start neovim.

I believe that the issue occurs because the data file uses the hash of the current working directory for to generate the file name.

local function fullpath(config)
    local h = hash(filename(config))
    return string.format("%s/%s.json", data_path, h)
end

When neovim loads, it uses the hash of the initial working folder, but when I open a new tab with a different working folder, it appears that the hash of the new working folder is used when saving the lists. So now lists are saved in a different file than the one loaded at startup.

Is there a reason for the hashing rather than one file?

The data file already contains data for multiple working directories already.

@stroiman
Copy link
Author

stroiman commented Jun 25, 2024

I made this change in my own fork, stroiman@f9a2ff2

Seems to work nicely (after 2 minutes of testing), but I wouldn't PR it as I don't understand the reasoning behind multiple files - but if you think this is a reasonable change, I'll gladly clean it up (all the callers still pass the config to the function - which really doesn't have to be a function anymore, as it no longer has any arguments)

@cosmicboots
Copy link

cosmicboots commented Jul 2, 2024

I think the deeper issue here is that Harpoon's Data isn't re-read from disc when changing directories. Because harpoon so heavily relies on vim's current working directory (a great decision imo), I believe it would be best to clear out harpoon's state and re-read it from the filesystem whenever vim's working directory changes.

I believe this gives the best of both worlds: allow directory changes and also keeping the separate state files (which I assume are needed for some of the more advanced features of v2).

I'll open a PR soon with the changes.

@stroiman
Copy link
Author

stroiman commented Jul 2, 2024

I think that there is something wrong in that statement.

While neovim has a working directory, each tab can have a separate working directory, and even windows can have separate directories.

In my case, I have a "project" open in one tab, and when I open my neovim config in another tab, that new tab has a different working directory, so the idea to "re-read it from the filesystem whenever vim's working directory changes" makes no sense. The directory didn't change. The new tab was just initialised with a different working dir.

@cosmicboots
Copy link

Ah, that's my bad. I misunderstood you're issue and thought it was the same issue I was having. I honestly have never used window local or tab local directory changes, so it hadn't even crossed my mind.

That being said, the changes I proposed should still work. I just tested with tabs (and windows) in different working directories, and the correct harpoon list is shown in relation to the tab/window's location.

@stroiman
Copy link
Author

stroiman commented Jul 3, 2024

I looked at your PR, and I think that it would actually ruin my flow.

So what I do is, while working on a project, I detect an common editing pattern that I want to optimize. I open init.lua in a new tab, setting the working directory for that tab to the .config/nvim folder, thus all navigation in that tab works nicely for editing the config. Once I've made that change, I re-source the config and go back to the project.

If I were to use your branch, once I get back to my original tab, the Harpoon marks would be gone.

But try to see if using my fork doesn't fix your problem? I think it should. Rather than detecting directory changes, it just stores all marks in one file rather than multiple files. But the config file is already grouped by working dir. So so far that change seems to work well.

But as I wrote before, I don't understand the intention behind multiple data files, which is why I didn't want to PR it yet.

@stroiman
Copy link
Author

stroiman commented Jul 3, 2024

Or maybe it should also react to TabEnter and WinEnter.

Multiple files is probably to avoid loading useless state into memory. When I work on project A, I don't need harpoon marks for project B in memory.

Maybe another solution is, when switching directory, rather than replace the state, merge the state. And maybe that's better than TabEnter, WinEnter - as those would be triggered a lot, and most of the times, there wouldn't be anything to do.

Then, when saving, don't use current dir. The data file is grouped by working dir, so rather than one save, save each working dir from the data separately.

@cosmicboots
Copy link

If I were to use your branch, once I get back to my original tab, the Harpoon marks would be gone.

This isn't true. The DirChanged neovim event is triggered when switching between windows with different effective directories. This means when you switch from the .config/nvim tab back to your project, the harpoon list will still contain your project's list.

@stroiman
Copy link
Author

stroiman commented Jul 3, 2024

You're right. I had actually tested it where I saw a different behaviour, but I must have done something wrong in the first test, so I guess your approach should work. It's getting late, here, I'll see if I can get to try your branch out tomorrow and see how that works on in my use case.

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

No branches or pull requests

2 participants