wfxr / forgit

:zzz: A utility tool powered by fzf for using git interactively.
MIT License
4.32k stars 136 forks source link

grc giving back error "git branch: command not found" #301

Closed cjappl closed 1 year ago

cjappl commented 1 year ago

When trying to run grc this morning, I got the error:

/Users/chrisapple/code/personal//forgit/conf.d/bin/git-forgit: line 568: git branch : command not found

I think there are two things going wrong here:

  1. The command should be git revert not git branch
  2. Something is fishy about how the command is being evaluated on line 592, not sure how to resolve it.

Repro'd on OSX

carlfriedrich commented 1 year ago

I can confirm that it should be git revert. I accidentally introduced this bug in #296, sorry for that.

Concerning the evaluation: does gcb work correctly for you? We're using the same mechanism via the git command in a variable there, without eval.

cjappl commented 1 year ago

Confusingly enough, (after I change it to git revert)

  1. gcb does work
  2. grc does not work

....

Any ideas what I could do to troubleshoot? Even completely simplifying like so has the same result:

@@ -534,7 +534,7 @@ _forgit_branch_delete() {
 _forgit_revert_commit() {
     _forgit_inside_work_tree || return 1
     local git_revert cmd opts files preview commits IFS
-    git_revert="git branch $FORGIT_REVERT_COMMIT_GIT_OPTS"
+    git_revert="git revert"
     [[ $# -ne 0 ]] && { $git_revert "$@"; return $?; }

     cmd="git log --graph --color=always --format='$_forgit_log_format' $* $_forgit_emojify"
@@ -565,7 +565,7 @@ _forgit_revert_commit() {

     [ ${#commits[@]} -eq 0 ] && return 1

-    $git_revert "${commits[@]}"
+    $git_revert
 }

 # git blame viewer
/Users/chrisapple/code/personal//forgit/conf.d/bin/git-forgit: line 568: git revert: command not found
carlfriedrich commented 1 year ago

Hmm, that's strange. What about ga, grh, gss, gsp, gclean, gcp, grb, gcf and gct? Those commands all don't use eval. Can you verify which of them work for you?

carlfriedrich commented 1 year ago

Maybe it has to do with the temporary changing of IFS within grc. We're doing this in gcp as well, though, so it would be interesting if that works.

carlfriedrich commented 1 year ago

FYI: I already fixed the wrong git call in #305.

cjappl commented 1 year ago

gcp works as intended ✅ gagcfgrhgssgspgcleangrbgcfgctgrc /Users/chrisapple/code/personal//forgit/conf.d/bin/git-forgit: line 568: git revert: command not found

hmm.... the plot thickens. Everything is perfectly good except for this one.

carlfriedrich commented 1 year ago

Crazy. 😳 Just to be sure: for testing you called all the commands without any arguments, using the interactive fzf selector to choose the objects, right?

cjappl commented 1 year ago

Yep that is correct, so

‘grc’

select something On Sun, May 7, 2023 at 8:40 AM, Tim ***@***.***(mailto:On Sun, May 7, 2023 at 8:40 AM, Tim < wrote: > Crazy. 😳 Just to be sure: for testing you called all the commands without any arguments, using the interactive fzf selector to choose the objects, right? > > — > Reply to this email directly, [view it on GitHub](https://github.com/wfxr/forgit/issues/301#issuecomment-1537472337), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ADMDXYYUEAGHJGGC2IZW2ILXE667FANCNFSM6AAAAAAXUTAUTQ). > You are receiving this because you authored the thread.Message ID: ***@***.***>
carlfriedrich commented 1 year ago

No, I meant exactly the opposite 😋:

‘grc’ \ select something \

If you're using \, you bypass the fzf selector and along with that the variable containing the git command as well.

cjappl commented 1 year ago

Both versions result in the same thing, using either to multiselect, or to just select one.

grc to enter the fzf selector

multiselect a bunch of things, or a single thing `grc` to enter the fzf selector select one thing
carlfriedrich commented 1 year ago

Funny thing: I can actually reproduce the error. Hadn't checked this, yet, haha. 🙈 I already found the issue, it really had to do with the setting of IFS, like I suspected. I am preparing a PR.