Conversation
Switching to a branch occupied by another worktree results in an error. Until proper support for work trees is released, I think this is a good stop gap.
carlfriedrich
left a comment
carlfriedrich
left a comment
There was a problem hiding this comment.
@iloveitaly Thanks a lot for the PR. Very clean implementation and big thumbs up for adding the unit test! Approved from my side. Will wait whether the other maintainers want to say something, otherwise I will merge this next week.
| | Option | Description | Default | | ||
| |--------------------------------------------|-----------------------------------------------|-----------------------------------------------| | ||
| | `FORGIT_CHECKOUT_BRANCH_EXCLUDE_WORKTREES` | exclude worktree branches in `gcb` | `true` | | ||
| | `FORGIT_LOG_FORMAT` | git log format | `%C(auto)%h%d %s %C(black)%C(bold)%cr%Creset` | | ||
| | `FORGIT_PREVIEW_CONTEXT` | lines of diff context in preview mode | 3 | | ||
| | `FORGIT_FULLSCREEN_CONTEXT` | lines of diff context in full-screen mode | 10 | | ||
| | `FORGIT_DIR_VIEW` | command used to preview directories | `tree` if available, otherwise `find` | |
There was a problem hiding this comment.
I think the rest of the table should be widened as well to keep the formatting consistent.
| } | ||
|
|
||
| _forgit_worktree_filter() { | ||
| if [[ "${FORGIT_CHECKOUT_BRANCH_EXCLUDE_WORKTREES:-true}" == "true" ]]; then |
There was a problem hiding this comment.
Ah, one more thing: we're evaluating this kind of env variables at a central place in the code (search for FORGIT_LOG_GRAPH_ENABLE for example). I think we should do it like that with this variable, too.
sandr01d
left a comment
sandr01d
left a comment
There was a problem hiding this comment.
Looks clean, just two minor questions/comments from my side.
| _forgit_worktree_filter() { | ||
| if [[ "${FORGIT_CHECKOUT_BRANCH_EXCLUDE_WORKTREES:-true}" == "true" ]]; then | ||
| # Worktree branches are prefixed with '+' in 'git branch' output | ||
| grep -v '^[+]' |
There was a problem hiding this comment.
From my understanding, the bracket expression isn't necessary or is there any reason we can't simply use:
| grep -v '^[+]' | |
| grep -v '^+' |
| } | ||
|
|
||
| _forgit_worktree_filter() { | ||
| if [[ "${FORGIT_CHECKOUT_BRANCH_EXCLUDE_WORKTREES:-true}" == "true" ]]; then |
There was a problem hiding this comment.
Is there actually a case where somebody would not want to exclude worktrees? If they don't work with checkout we might want to always exclude them...
|
As @carlfriedrich says, kudos for adding the unit test! I love seen bashunit being use and getting adoption! It feels so good helping the community. Cheers! |
4 participants
Check list
Description
If a branch is used by a git worktree it cannot be switched to and is
prefixed with a
+. This excludes these branches fromgcbto avoiderroring out when they are selected.
Related to #399
Type of change
Test environment