Feat: add interactive git worktree operations #402

suft · 2024-11-03T21:48:27Z

Check list

I have performed a self-review of my code
I have commented my code in hard-to-understand areas
I have made corresponding changes to the documentation

Description

I recently adopted using multiple fixed worktrees as part of my workflow to help with productivity.
Each worktree is used for a different type of concurrent activity:

main for looking at the pristine code
work for looking at my code
review for looking at someone else’s code
background for my computer to look at my code
- operates in the detached head state
scratch for everything else

. (bare repo)
├── background/ (* worktree)
├── config
├── description
├── HEAD
├── hooks/
├── info/
├── logs/
├── main/ (* worktree)
├── objects/
├── packed-refs
├── refs/
├── review/ (* worktree)
├── rr-cache/
├── scratch/ (*worktree)
├── work/(* worktree)
└── worktrees/

Example of worktrees in a bare clone

Another approach is to use worktrees as a replacement of, or a supplement to git branches. Instead of switching branches, you just change directories. So that would involve creating a new worktree and branch, then delete the worktree upon merging.

Worktree Operations (should we stick with these abbreviations?)

locking a worktree - gwl
unlocking a worktree - gwu
removing a worktree - gwr
jumping to a worktree - gwj
- similar to switching branches
- involves a combination of selecting a result of git worktree list and cd into that result (not a native git operation)

Screenshots

bash (repo with short history) - gwj
FORGIT_WORKTREE_PREVIEW_GIT_OPTS='--oneline --graph --decorate --color'

fish (repo with mid-length history) - gwj
FORGIT_WORKTREE_PREVIEW_GIT_OPTS='--oneline --graph --decorate --color'

bash (repo with mid-length history) - gwj
FORGIT_WORKTREE_PREVIEW_GIT_OPTS='--oneline --graph --decorate --color --max-count=100'

Closes #399.

Type of change

Test environment

Shell
- bash
- zsh
- fish
OS
- Linux
- Mac OS X
- Windows
- Others:

carlfriedrich · 2024-11-04T09:29:50Z

@suft Thanks for your contribution! Is this ready for review or did you make it a draft on purpose?

suft · 2024-11-04T11:16:36Z

@carlfriedrich I made it draft on purpose. Still have a few things to adjust.

suft · 2024-11-10T16:24:35Z

bin/git-forgit

+    [[ "$sha" == "(bare)" ]] && return
+    # the trailing '--' ensures that this works for branches that have a name
+    # that is identical to a file
+    git log "$sha" "${_forgit_log_preview_options[@]}" --


I should probably add preview options FORGIT_WORKTREE_PREVIEW_GIT_OPTS (like Add *_PREVIEW_GIT_OPTS variables #396)

I've noticed this can run a bit slow (on some computers) if you have a repo with a long history because it attempts to get the entire history for the branch checked out in that worktree

It would be good to see what others think when they test this out

Would be quicker if I limit the log entries in the worktree preview options

bin/git-forgit

sandr01d

Thanks for you work @suft! I think this is going to be a nice improvement. There are a few things that need to be adjusted. In some of my comments I've pinged the other maintainers for their opinions. Please hold back on implementing those, as those comments are opinionated and I'd like to have feedback from the others first before deciding in which direction to go.

bin/git-forgit

sandr01d · 2024-11-12T13:45:10Z

bin/git-forgit

+
+    tree=$(git worktree list | awk '{print $1}' | FZF_DEFAULT_OPTS="$opts" fzf)
+    [[ -z "$tree" ]] && return 1
+    echo "$tree"


I think it would be cleaner to cd into the directory here instead of doing so in forgit.plugin.zsh and forgit.plugin.fish.

Using cd directly in the function only changes the current directory in the script child process, not in the terminal process where the script is called

Any preference between cd and pushd?

This is tricky, because the actual change of the directory is currently implemented in the shell plugin. Some users (e.g. me) do not use forgit with the plugin but in the form of a subcommand of git. Calling git forgit worktree_jump won't work at the moment, though. IMO we should find a way to make this work correctly in both usecases, or otherwise rename the function (because right now it actually does not jump).

We could rename the function to worktree_select - gws
(but also offer the gwj alias which combines cd and worktree_select)

Given that there is no way to manipulate the directory of the parent process, I think this is the best compromise we can achieve. @carlfriedrich WDYT?

Seems resonable to me. We should add an explaining line on this to the documentation, though.

sandr01d · 2024-11-12T13:49:50Z

bin/git-forgit

+        $FORGIT_WORKTREE_LOCK_FZF_OPTS
+    "
+
+    tree=$(git worktree list | awk '{print $1}' | grep -v "(bare)" | FZF_DEFAULT_OPTS="$opts" fzf)


What exactly is grep -v "(bare)" filtering out? I'm new to the whole worktree thing, could you please elaborate?
There are also more things we need to filter out here:

The root worktree (which can not be locked)

Worktrees that are already locked

When using worktrees with a bare clone, you automatically get the git-dir as an entry in the
git worktree list output (even though its not really a worktree).
So for certain operations we want to filter it out (like locking, unlocking, removing, ...).
Good catch on filtering already locked (totally slipped my mind).

bin/git-forgit

sandr01d · 2024-11-12T13:55:11Z

bin/git-forgit

+
+    opts="
+        $FORGIT_FZF_DEFAULT_OPTS
+        +s -m --tiebreak=index --header-lines=1


I'm not a fan of the header line here. I would prefer to have no header line and simply not display the root worktree with this command. This also applies to the other functions using --header-lines=1. Would be interested to know what other maintainers think @cjappl @carlfriedrich @wfxr.

This is the way we do it with all other forgit functions as well, so I would approve it like this. @sandr01d are you questioning this principle in general for all forgit functions?

This is the way we do it with all other forgit functions as well

Weird, I've never noticed. In this case I'm fine with keeping it in this PR as well.

bin/git-forgit

sandr01d · 2024-11-12T14:24:55Z

bin/git-forgit

+
+    tree=$(git worktree list | awk '{print $1}' | grep -v "(bare)" | FZF_DEFAULT_OPTS="$opts" fzf)
+    [[ -z "$tree" ]] && return 1
+    _forgit_git_worktree_lock "$tree"


It would be nice to allow passing in additional arguments as we do with other functions by propagating the function arguments, so that gwl --reason <STRING> works as expected.
This also applies to the other functions.

carlfriedrich · 2024-11-15T12:23:25Z

@suft Great implementation so far, @sandr01d great review. This will be a good addition to forgit.

sandr01d

Thanks for the changes @suft, looks good to me so far. Let me summarize the things that still need to be addressed:

The default preview should look the same as for other commands that use git log (Feat: add interactive git worktree operations #402 (comment))
_forgit_worktree_jump does not change the directory when forgit is used as a git subcommand (Feat: add interactive git worktree operations #402 (comment))
The root worktree should not be selectable with gwl. We can make it non-interactable with --header-lines=1 and also bring this back in the other functions that you used this with. Don't mind my earlier comment regarding this. (Feat: add interactive git worktree operations #402 (comment))
We should add completions for the new commands (Feat: add interactive git worktree operations #402 (comment))
We should allow passing additional arguments to the underlying git commands (Feat: add interactive git worktree operations #402 (comment)). The logic for detecting whether the git command should be executed immediately without using fzf needs to be adjusted to do so. Instead of testing if any arguments were passed, we need to test whether non flag arguments were passed. You can use _forgit_contains_non_flags to do so.

bin/git-forgit

cjappl · 2024-11-15T19:13:33Z

(it looks like there is very thorough reviewing going on here, please ping me if you need another pair of eyes, otherwise I completely defer to @sandr01d and @carlfriedrich )

suft · 2024-12-18T20:03:15Z

Thanks for the changes @suft, looks good to me so far. Let me summarize the things that still need to be addressed:

The default preview should look the same as for other commands that use git log (feat: add interactive git worktree operations #402 (comment))

_forgit_worktree_jump does not change the directory when forgit is used as a git subcommand (feat: add interactive git worktree operations #402 (comment))

The root worktree should not be selectable with gwl. We can make it non-interactable with --header-lines=1 and also bring this back in the other functions that you used this with. Don't mind my earlier comment regarding this. (feat: add interactive git worktree operations #402 (comment))

We should add completions for the new commands (feat: add interactive git worktree operations #402 (comment))

We should allow passing additional arguments to the underlying git commands (feat: add interactive git worktree operations #402 (comment)). The logic for detecting whether the git command should be executed immediately without using fzf needs to be adjusted to do so. Instead of testing if any arguments were passed, we need to test whether non flag arguments were passed. You can use _forgit_contains_non_flags to do so.

The root worktree doesn't show up as the first selection, so --header-lines=1 will affect a different worktree (which I'm guessing is not the behaviour we want), unless we reverse the fzf item list.

sandr01d

Thanks again for your work @suft. There are a few more things that need to be addressed, but I think we're heading in the right direction 🙂 I think there might have been a misunderstanding regarding argument parsing, sorry if I didn't make it clear enough what I meant previously. I provided an example this time. In case any questions come up, please don't hesitate to ping me.

README.md

sandr01d · 2024-12-21T15:58:12Z

bin/git-forgit

+    local tree opts reason
+    if [[ $# -ne 0 ]]; then
+        if [[ "$1" == "--reason" ]]; then
+            if [[ -z "$2" ]]; then
+                _forgit_warn "Option \`--reason' requires a value"
+                return 1
+            fi
+            reason="$2"
+            shift 2
+        fi
+
+        if [[ $# -eq 0 ]] || _forgit_contains_non_flags "$@"; then
+            git worktree lock "$@" --reason "$reason"
+            return $?
+        fi
+    fi
+
+    opts="
+        $FORGIT_FZF_DEFAULT_OPTS
+        +s +m --tiebreak=index
+        --preview=\"$FORGIT worktree_preview {1}\"
+        $FORGIT_WORKTREE_LOCK_FZF_OPTS
+    "
+
+    tree=$(git worktree list | grep -vE "\(bare\)|locked" | awk '{print $1}' | FZF_DEFAULT_OPTS="$opts" fzf)
+    [[ -z "$tree" ]] && return 1
+
+    if [[ -z "$reason" ]]; then
+        _forgit_git_worktree_lock "$tree"
+    else
+        _forgit_git_worktree_lock "$tree" --reason "$reason"
+    fi


I think we could simplify this by not parsing the arguments and instead simply propagating them. This also allows us to stay up-to-date without any changes in case git introduces new arguments in the future.

Suggested change

local tree opts reason

if [[ $# -ne 0 ]]; then

if [[ "$1" == "--reason" ]]; then

if [[ -z "$2" ]]; then

_forgit_warn "Option \`--reason' requires a value"

return 1

fi

reason="$2"

shift 2

fi

if [[ $# -eq 0 ]] || _forgit_contains_non_flags "$@"; then

git worktree lock "$@" --reason "$reason"

return $?

fi

fi

opts="

$FORGIT_FZF_DEFAULT_OPTS

+s +m --tiebreak=index

--preview=\"$FORGIT worktree_preview {1}\"

$FORGIT_WORKTREE_LOCK_FZF_OPTS

"

tree=$(git worktree list | grep -vE "$bare$|locked" | awk '{print $1}' | FZF_DEFAULT_OPTS="$opts" fzf)

[[ -z "$tree" ]] && return 1

if [[ -z "$reason" ]]; then

_forgit_git_worktree_lock "$tree"

else

_forgit_git_worktree_lock "$tree" --reason "$reason"

fi

local tree opts

if [[ $# -ne 0 ]] && _forgit_contains_non_flags "$@"; then

git worktree lock "$@"

return $?

fi

opts="

$FORGIT_FZF_DEFAULT_OPTS

+s +m --tiebreak=index

--preview=\"$FORGIT worktree_preview {1}\"

$FORGIT_WORKTREE_LOCK_FZF_OPTS

"

tree=$(git worktree list | grep -vE "$bare$|locked" | awk '{print $1}' | FZF_DEFAULT_OPTS="$opts" fzf)

[[ -z "$tree" ]] && return 1

_forgit_git_worktree_lock "$tree" "$@"

sandr01d · 2024-12-21T15:59:49Z

bin/git-forgit

+    _forgit_inside_work_tree || _forgit_inside_git_dir || return 1
+    local worktree_list tree opts force_flags
+
+    force_flags=()


Same here, I would prefer simply propagating the arguments as we do everywhere else.

sandr01d · 2024-12-21T16:25:57Z

completions/_git-forgit

+_git-worktrees() {
+    _alternative "worktrees:worktree:($(git worktree list --porcelain | awk '/worktree/ {print $2}'))"
+}


This always suggests me refs/heads/worktrees which is not a worktree. Also consider using -z together with --porcelain. From the git man page:

--porcelain
With list, output in an easy-to-parse format for scripts. This format will remain stable
across Git versions and regardless of user configuration. It is recommended to combine this
with -z. See below for
-z
Terminate each line with a NUL rather than a newline when --porcelain is specified with
list. This makes it possible to parse the output when a worktree path contains a newline
character.

The same applies to the bash completions.

sandr01d · 2024-12-21T16:58:07Z

completions/git-forgit.fish

@@ -42,6 +42,10 @@ complete -c git-forgit -n __fish_forgit_needs_subcommand -a reset_head -d 'git r
 complete -c git-forgit -n __fish_forgit_needs_subcommand -a revert_commit -d 'git revert commit selector'
 complete -c git-forgit -n __fish_forgit_needs_subcommand -a stash_show -d 'git stash viewer'
 complete -c git-forgit -n __fish_forgit_needs_subcommand -a stash_push -d 'git stash push selector'
+complete -c git-forgit -n __fish_forgit_needs_subcommand -a worktree_select -d 'git worktree selector'


I get error messages that look like this when trying to execute one of the commands in fish when inside a worktree:

forgit: 'worktree_lock' is not a valid forgit command. The following commands are supported: add blame branch_delete checkout_branch checkout_commit checkout_file checkout_tag cherry_pick cherry_pick_from_branch clean diff fixup ignore log rebase reset_head revert_commit stash_show stash_push

@cjappl is our fish expert in case you need some help with this.

sandr01d · 2024-12-21T17:16:31Z

The root worktree doesn't show up as the first selection, so --header-lines=1 will affect a different worktree (which I'm guessing is not the behaviour we want), unless we reverse the fzf item list.

I'm not exactly sure what you mean. git worktree list always seems to give me the root worktree as the first line and git worktree list | fzf --header-lines 1 does not allow me to select the root worktree as I'd expect. Could you please provide an example on how to reproduce this?

suft force-pushed the worktrees branch from 44ee72f to 6d5dd39 Compare November 5, 2024 01:28

suft mentioned this pull request Nov 5, 2024

Feature Request: Add git worktree switcher like gcb #399

Open

10 tasks

suft commented Nov 10, 2024

View reviewed changes

bin/git-forgit Outdated Show resolved Hide resolved

suft force-pushed the worktrees branch from 6d5dd39 to 4005f22 Compare November 10, 2024 17:52

feat: add interactive git worktree operations

2218cbd

suft force-pushed the worktrees branch from 4005f22 to cae816c Compare November 10, 2024 19:40

docs: update README to include worktree operations

525441f

suft force-pushed the worktrees branch from cae816c to 525441f Compare November 10, 2024 19:44

suft marked this pull request as ready for review November 10, 2024 19:45

cjappl requested review from sandr01d, carlfriedrich, wfxr and cjappl November 11, 2024 12:03

sandr01d requested changes Nov 12, 2024

View reviewed changes

fix: typos

7e50ba6

suft requested a review from sandr01d November 14, 2024 00:32

sandr01d reviewed Nov 15, 2024

View reviewed changes

bin/git-forgit Outdated Show resolved Hide resolved

suft added 2 commits November 16, 2024 14:01

fix: address reviewer comments

a339901

refactor: rename worktree_jump to worktree_select

2603418

suft marked this pull request as draft November 18, 2024 00:09

suft force-pushed the worktrees branch from b680b3e to 2603418 Compare November 18, 2024 00:15

suft added 2 commits November 23, 2024 11:37

feat: add fish shell completions

e2764ec

feat: add zsh shell completions

36fd505

suft added 3 commits December 1, 2024 12:40

feat: add bash shell completions

bcdd3a1

feat: add flag parsing for git worktree lock

a0dc127

feat: add flag parsing for git worktree remove

99d05c5

suft force-pushed the worktrees branch from e0e6125 to 99d05c5 Compare December 18, 2024 17:09

suft marked this pull request as ready for review December 18, 2024 19:08

suft requested a review from sandr01d December 18, 2024 19:08

suft marked this pull request as draft December 18, 2024 19:24

suft marked this pull request as ready for review December 18, 2024 19:50

suft changed the title ~~feat: add interactive git worktree operations~~ Feat: add interactive git worktree operations Dec 18, 2024

sandr01d requested changes Dec 21, 2024

View reviewed changes

docs: include gws and gwj aliases together

6a2257c

suft marked this pull request as draft December 24, 2024 20:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: add interactive git worktree operations #402

Feat: add interactive git worktree operations #402

suft commented Nov 3, 2024 •

edited

Loading

carlfriedrich commented Nov 4, 2024

suft commented Nov 4, 2024

suft Nov 10, 2024 •

edited

Loading

sandr01d left a comment

sandr01d Nov 12, 2024

suft Nov 13, 2024

carlfriedrich Nov 15, 2024

suft Nov 16, 2024

sandr01d Nov 17, 2024

carlfriedrich Nov 18, 2024

sandr01d Nov 12, 2024

suft Nov 13, 2024

sandr01d Nov 12, 2024

carlfriedrich Nov 15, 2024

sandr01d Nov 15, 2024

sandr01d Nov 12, 2024

carlfriedrich Nov 15, 2024

carlfriedrich commented Nov 15, 2024

sandr01d left a comment

cjappl commented Nov 15, 2024

suft commented Dec 18, 2024

sandr01d left a comment

sandr01d Dec 21, 2024

sandr01d Dec 21, 2024

sandr01d Dec 21, 2024

sandr01d Dec 21, 2024

sandr01d commented Dec 21, 2024

Feat: add interactive git worktree operations #402

Are you sure you want to change the base?

Feat: add interactive git worktree operations #402

Conversation

suft commented Nov 3, 2024 • edited Loading

Check list

Description

Screenshots

Type of change

Test environment

carlfriedrich commented Nov 4, 2024

suft commented Nov 4, 2024

suft Nov 10, 2024 • edited Loading

Choose a reason for hiding this comment

sandr01d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlfriedrich commented Nov 15, 2024

sandr01d left a comment

Choose a reason for hiding this comment

cjappl commented Nov 15, 2024

suft commented Dec 18, 2024

sandr01d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandr01d commented Dec 21, 2024

suft commented Nov 3, 2024 •

edited

Loading

suft Nov 10, 2024 •

edited

Loading