Enable upstream tracking

[rbmarliere]

As previously discussed in PR 20 and now mentioned in issue 30, dealing with remote tracking trees currently does not have a good user experience. This pull request addresses this, while also adding a necessary feature to make that experience even smoother: switching directories back to the bare repository. Finally, a few minor fixes.

There are still some rough edges mentioned in some new TODOs and other minor stuff like properly announcing when the jobs ended, but those can be addressed later.

[polarmutex]

I will look at this soon

[tvsfx]

@rbmarliere ; thanks for taking a stab at implementing this; I've been sorely wanting this functionality as of late :)
Sorry I'm so late to look at your code, but I had a couple questions/comments still (even though it's been merged already in the meantime, so I cant review). Let me know if I misunderstood something, I didn't study the code in depth.

1. It feels like there's a disconnect in the code between the telescope and the git-worktree logic; the user is asked whether they want to track a branch, and they declare intent by selecting whether the want to select or create a branch (perhaps select shouldn't create as a fallback?), but this information is not stored in the core worktree logic itself. This causes duplication and risks inconsistency (e.g. create_branch could coincidentally use an existing branch, afaict). It might help to explicitly list all scenario's we want to support (tracking or not, creation vs selection, detach, etc.), and figure out which scenario we're in, in one location, once and for all (create_worktree_job now does this internally, from a bunch of inputs).
2. Related to my first point; I have the impression that the has_worktree logic does not account for the scenario where a name clash occurs when a newly created local branch (created through --track) already exists. In this case the worktree logic would fail.

3. git-worktree.nvim/lua/telescope/_extensions/git_worktree.lua

   Line 101 in 6e1fa15

   git_worktree.switch_worktree(nil)

   -> why do you need to proactively switch out of all worktrees when you delete one? Am I missing something, or does this make it so you have to explicitly select your previous worktree again when you delete an unrelated one? I'd expect the deletion to either just fail (I think that's fine; the user can easily move to another worktree) or only prompt the user if a clash is actually detected.
4. On a similar note, I personally don't love the idea of switching to the nil worktree; it causes confusing behaviour (e.g. switch_worktree can now switch to something that's not a worktree) and it creates quite a few special cases throughout the code that look like the following (which would be avoided by prompting for a switch at the right time or just outright failing):

   if selection == nil then
       return
   end

5. What is this local prefix and why is it necessary?

   git-worktree.nvim/lua/git-worktree/worktree.lua

   Line 133 in bac72c2

   branch = 'local/' .. branch

   Is this a naming convention you introduced? Doesn't this cause issues when the user selects a remote tracking branch that is already being tracked locallly? I'd either follow git and reuse the remote's name, or use the user-provided one.
6. Intuitively, I wouldn't have expected you to need three calls to hash_branch here (only 2). It's strange to me that upstream can be overwritten by branch still (but see point 1.)
7. You substitute the remotes/ prefix away in has_branch but also use the --track option when creating a worktree. Do you need both?

[rbmarliere]

@rbmarliere ; thanks for taking a stab at implementing this; I've been sorely wanting this functionality as of late :) Sorry I'm so late to look at your code, but I had a couple questions/comments still (even though it's been merged already in the meantime, so I cant review). Let me know if I misunderstood something, I didn't study the code in depth.

Thanks for the review! As a disclaimer I'm not very knowledgeable with Lua and plugins in general and it was indeed mostly a hack with lots of trial and error... But it somewhat worked for me and thus decided to share upstream.

1. It feels like there's a disconnect in the code between the `telescope` and the `git-worktree` logic; the user is asked whether they want to track a branch, and they declare intent by selecting whether the want to [select](https://github.com/polarmutex/git-worktree.nvim/blob/6e1fa150546b9cd2da6985f0b959f3ad4f7637aa/lua/telescope/_extensions/git_worktree.lua#L172) or [create](https://github.com/polarmutex/git-worktree.nvim/blob/6e1fa150546b9cd2da6985f0b959f3ad4f7637aa/lua/telescope/_extensions/git_worktree.lua#L158) a branch (perhaps select shouldn't create as a fallback?), but this information is not stored in the core worktree logic itself. This causes duplication and risks inconsistency (e.g. `create_branch` could coincidentally use an existing branch, afaict). It might help to explicitly list all scenario's we want to support (tracking or not, creation vs selection, detach, etc.), and figure out which scenario we're in, in one location, once and for all (`create_worktree_job` now does this internally, from a bunch of inputs).

The problem I faced was wanting to create a branch named "foo" but that string still yielded results like "remote1/foo" so I needed another action (mapped to tab) that would create it anyway. Could be better indeed. But the lines you linked has to do with the branch to create, not track. Tracking comes later.

2. Related to my first point; I have the impression that the `has_worktree` logic does not account for the scenario where a name clash occurs when a newly created local branch (created through `--track`) already exists. In this case the worktree logic would fail.

If I understood correctly, you'd be trying to create a tree in path "tree2" checking out a branch already in use in "tree1", if so then it fails silently due to:

16:39:56 rbmarliere@opensuse /mnt/ext/src/rbmarliere/git-worktree.nvim BARE:main
$ git worktree list
/mnt/ext/src/rbmarliere/git-worktree.nvim        (bare)
/mnt/ext/src/rbmarliere/git-worktree.nvim/devel  edd16a7da828 [devel]
/mnt/ext/src/rbmarliere/git-worktree.nvim/main   bac72c240b6b [main]
16:40:13 rbmarliere@opensuse /mnt/ext/src/rbmarliere/git-worktree.nvim BARE:main
$ git worktree add devel2 devel
Preparing worktree (checking out 'devel')
fatal: 'devel' is already used by worktree at '/mnt/ext/src/rbmarliere/git-worktree.nvim/devel'

3. https://github.com/polarmutex/git-worktree.nvim/blob/6e1fa150546b9cd2da6985f0b959f3ad4f7637aa/lua/telescope/_extensions/git_worktree.lua#L101
    -> why do you need to proactively switch out of all worktrees when you delete one? Am I missing something, or does this make it so you have to explicitly select your previous worktree again when you delete an unrelated one? I'd expect the deletion to either just fail (I think that's fine; the user can easily move to another worktree) or only prompt the user if a clash is actually detected.

The reason behind this is to avoid errors when the user deletes the currently selected tree and avoid nested paths when creating a new tree while being "within" (selected) a tree already. That way the path is always relative to the bare repo basedir.

4. On a similar note, I personally don't love the idea of switching to the `nil` worktree; it causes confusing behaviour (e.g. `switch_worktree` can now switch to something that's not a worktree) and it creates quite a few special cases throughout the code that look like the following (which would be avoided by prompting for a switch at the right time or just outright failing):
   ```lua
   if selection == nil then
       return
   end
   ```

That's a fair point.

5. What is this `local` prefix and why is it necessary? https://github.com/polarmutex/git-worktree.nvim/blob/bac72c240b6bf1662296c31546c6dad89b4b7a3c/lua/git-worktree/worktree.lua#L133
    Is this a naming convention you introduced? Doesn't this cause issues when the user selects a remote tracking branch that is already being tracked locallly? I'd either follow git and reuse the remote's name, or use the user-provided one.

I wasn't happy with this one either. The problem is, when creating and being presented the branches list to choose, you might either create a new one or select a remote branch. If you select a remote branch, this is what would happen:

$ git worktree add rbmarliere/track_upstream -b rbmarliere/track_upstream --track rbmarliere/track_upstream
Preparing worktree (new branch 'rbmarliere/track_upstream')
branch 'rbmarliere/track_upstream' set up to track 'rbmarliere/track_upstream'.
warning: refname 'rbmarliere/track_upstream' is ambiguous.
HEAD is now at 0f6da2603450 feat: Enable user to create detached worktree

But instead, this is what happens:

$ git worktree add rbmarliere/track_upstream -b local/rbmarliere/track_upstream --track rbmarliere/track_upstream

It works for me but I agree it's not optimal. Another option would be to limit the branch list to local/remote and split the functionalities. So, one action for each. But that's not great either. I'm very interested in suggestions here.

6. Intuitively, I wouldn't have expected you to need three calls to `hash_branch` [here](https://github.com/polarmutex/git-worktree.nvim/blob/bac72c240b6bf1662296c31546c6dad89b4b7a3c/lua/git-worktree/worktree.lua#L129-L137) (only 2). It's strange to me that `upstream` can be overwritten by `branch` still (but see point 1.)

Good point. I did it just to get over with and avoid more refactoring... :)

7. You substitute the `remotes/` prefix away in `has_branch` but also use the `--track` option when creating a worktree. Do you need both?

I didn't follow this point. You need to pass a valid ref to in --track.

Please do share your thoughts! We can always improve things. There's a couple of bugs that I've been hitting but still haven't had the time to dig into them.

[rbmarliere]

After some more consideration and testing I submitted a few PRs addressing some points of the discussion:

#36
#37
#38

[rbmarliere]

FWIW I decided to write my own extension.