git-worktree.nvim icon indicating copy to clipboard operation
git-worktree.nvim copied to clipboard

Fails to find correct git_worktree_root

Open TamaMcGlinn opened this issue 4 years ago • 16 comments

I open vim in my home directory, then change the directory to that of a git worktree within a bare repository. Then I try to switch to a different worktree, but git-worktree falls back to the :Explore command because it can't find the file - after adding some debugging print statements in init.lua I find that it is trying to find it inside my home directory, because local git_worktree_root = vim.loop.cwd() is called when the plugin is loaded.

TamaMcGlinn avatar May 03 '21 13:05 TamaMcGlinn

was this addressed by #29 ?

I feel like you mentioned this.

ThePrimeagen avatar May 03 '21 14:05 ThePrimeagen

No. And I just updated but that made things worse. Now it doesn't work even if I start vim in the bare git repo directory, because now git_worktree_root is nil rather than the directory which vim starts in.

TamaMcGlinn avatar May 03 '21 14:05 TamaMcGlinn

Sorry for the duplicate issue; I did not find #29 but this is exactly that issue. It's just not solved yet.

TamaMcGlinn avatar May 03 '21 14:05 TamaMcGlinn

I am confused , is this solved or is there an issue on the latest master now? the latest master is detecting the git root on my system just want to make sure I did not miss a corner case.

polarmutex avatar May 03 '21 15:05 polarmutex

I may also be missing something.

My testing I can create, delete, and switch trees as of now. I use bare repos.

ThePrimeagen avatar May 03 '21 15:05 ThePrimeagen

Should we also move this convo to #29?

ThePrimeagen avatar May 03 '21 15:05 ThePrimeagen

I'm not sure where to move this either; I was thinking #30 at first, since it was the same issue, but then I updated to the latest master (cdcc26d Merge pull request #32 from rudotriton/prompt-title) and now switching doesn't work even if I do start vim in the git root directory. So I guess this really is a new issue, and I will reopen.

I would love to add a failing unit test in a PR, but I don't understand how to even run the unit tests at all; you require("git-worktree") and I guess that should read git-worktree/init.lua but my lua installation just says it can't find git-worktree.lua in various directories.

I will keep debugging this, but in terms of repro steps it's as simple as 'try to change worktrees' using :lua require('telescope').extensions.git_worktree.git_worktrees(), so it is a mystery to me why I am the only one with the problem.

TamaMcGlinn avatar May 04 '21 06:05 TamaMcGlinn

I've temporarily fixed this in my own vimrc with this:

fu! Switch_Worktree() abort
  lua require('git-worktree')._find_git_root_job()
  lua require('telescope').extensions.git_worktree.git_worktrees()
endfunction

nnoremap <leader>gww :call Switch_Worktree()<CR>

You are still assuming that the directory which vim initially starts in is within the git project; I tend to start nvim straight in my home directory, and have a Startify menu which means one number press brings me into both file and directory of one of the last places I was editing before. I also change directory all the time when moving about from project to project, so it doesn't make sense to me to call find_git_root_job() once at startup and then never again.

TamaMcGlinn avatar May 04 '21 06:05 TamaMcGlinn

I also noticed that when you have 3 worktrees involved, where one is the current CD path, you have a file open which is in the second, but you want to switch to a third one, then git_worktrees() manages to switch the directory, but is not able to open the file afterwards, so it opens the root directory instead. Really the current directory should not be relevant; rather than doing a string substitution to get the relative path from the root of the worktree to the file, we should probably just ask git: !git rev-parse --show-prefix % ?

TamaMcGlinn avatar May 04 '21 08:05 TamaMcGlinn

I see the issue now, calling any of the functions requires you to be in a git repo. with #37 , I want to discuss and think about the best way to add this in. For example, if we use just branch names, we will have no way to know the git repo unless the cwd was a the git repo we want to use.

for what is currently implemented(using path not branch) will the following be good?

For Switch and Delete

  • If the current cwd is a git repo, any call should use that directory
  • If the current cwd is not a git repo, check the path is in a git repo and use that if so For Create
  • Always assume we are in a git repo

Then to support #37, we could create new functions that take a branch name and require to be in a git repo

Thoughts?

polarmutex avatar May 04 '21 13:05 polarmutex

I don't believe the current working directory is ever relevant, and I certainly wouldn't want it to influence the decision of where to jump to. For instance, fugitive also only looks at the actual file you have open, and completely ignores your working directory.

Is it acceptable to add a dependency on the fugitive plugin?

let root_dir = system(fugitive#repo().git_command() . ' rev-parse --git-dir')

Otherwise, grabbing the internals of fugitive would also be more efficient that this; that system call is not necessary since fugitive already knows for each buffer what the git-dir is.

TamaMcGlinn avatar May 04 '21 14:05 TamaMcGlinn

changed so it finds the git root when calling create. switch, or delete.

polarmutex avatar May 04 '21 18:05 polarmutex

There is a danger when considering the current file. Which in some sense I would argue is wrong.

You could end up changing all sorts of things like that.

Perhaps this is a mode of operation issue. I will never open up files all around, I'll just be switching worktrees as I do my work. I always open my vim experience from the root, and the starting place, the cwd, is the git root at all times except when I shortcut and go to the branch I really want (which btw still works).

The reason why I don't like only considering current file is that I sometimes edit my vimrc. That is in completely different project and I wont want it to influence anything.

@TamaMcGlinn I think that a unit test showing the break would be the best possible solution to this. If you make a PR with a breaking unit test I am positive I or polar can have a more robust talk about this because I ma still slightly confused of the exact use case you are going for (even when reading it). I know, I am big dumb, and want to support you :)

ThePrimeagen avatar May 05 '21 14:05 ThePrimeagen

My current workaround is to call fugitive's Gcd prior to switching worktree. It works, but the on_tree_update function I pass does not. I am trying to make it also jump to the same place in the file as where I was before:

fu! Switch_Worktree() abort
  silent execute 'Gcd'
  lua << EOF
  local linenr = vim.api.nvim_win_get_cursor(0)[1]
  local Worktree = require("git-worktree")
  Worktree._find_git_root_job()
  Worktree.on_tree_update = (function(op, metadata)
    vim.cmd('execute "normal! ' .. linenr .. 'G"')
  end)
  require('telescope').extensions.git_worktree.git_worktrees()
EOF
endfunction

nnoremap <leader>gww :call Switch_Worktree()<CR>

the on_tree_update example in the README isn't very helpful, it doesn't seem to show actually reassigning the funcref. I think it would really helpful to be able to pass a lambda to git_worktrees() which is executed just once; my current workaround is actually reassigning a persistent action every time, which isn't really what I mean.

TamaMcGlinn avatar May 17 '21 15:05 TamaMcGlinn

@TamaMcGlinn Your best bet is to use telescope-project andshell out the check if :lcd switched to a worktree and setting it accordingly via script inside git-worktree.

This could require a commit to provide a function that sets the root_dir for git-worktree.

Unfortunately telescope-project has no support for checking if a path is a bare git yet, but it should work nevertheless. And git also does not support finding the root dir inside bare repos, but it is trivial to combine from the aforement checks.

matu3ba avatar Sep 25 '21 22:09 matu3ba

Sorry, I've just been so busy to try to take care of these things. I'll try to take a look at it this week. But I can feel that nfts are going to creep up and take all my time.

ThePrimeagen avatar Sep 25 '21 23:09 ThePrimeagen