aaaaahaaaaa commented 5 years ago

12

atweiden commented 5 years ago

Thank you for providing this PR. It is almost exactly what I wanted to see.

What is your reasoning for deleting the existing contents of fzf-extras.zsh? Did fzf-locate-widget not work? Is the if [[ $- =~ i ]] control flow unnecessary?

aaaaahaaaaa commented 5 years ago

I've added back the control flow statement. Regarding fzf-locate-widget, I deleted it because it was updated more than 3 years ago, is not part of the main bash script and more importantly I couldn't test it. I'll add it back if you believe it should stay.

atweiden commented 5 years ago

Note I got the zsh content from https://github.com/junegunn/fzf/wiki/Examples#locate.

Can you explain what went wrong in testing the fzf-locate-widget function? I don't necessarily object to deleting the original contents, but I don't agree that fzf-extras.sh and fzf-extras.zsh (and fzf-extras.fish in the future) should have equivalent features. Sometimes that won't be possible.

Regarding the control flow, I think it's fine to assume users setup ~/.zshrc itself not to source fzf-extras.zsh if their shell isn't running interactively. This is how my ~/.bashrc works for instance.

Come to think of it, I'm not sure why I originally added that control flow statement in the initial commit. I was probably just copy/pasting it from upstream, which isn't a good reason for it to be there.

aaaaahaaaaa commented 5 years ago

I couldn't test it because my locate database wasn't built locally. I now tested it, it's working and I've added it back.

atweiden commented 5 years ago

I've integrated your PR into https://github.com/atweiden/fzf-extras/tree/zsh. Does it look ok?

benoittgt commented 5 years ago

Thanks a lot for working on this. I love zsh and even if I'm not very good in shell I would love to have more script on zsh.

I tried few functions in the PR and have comments

`fbr`

I have issue using your fbr. Especially on this part

  branch="$(
    echo "$branches" \
      | fzf-tmux -d "$((2 + "$num_branches"))" +m
  )"
zsh: bad math expression: operand expected at `""'

I have a working version I use all the time that is similar but uglier https://github.com/benoittgt/dotfiles_osx/blob/master/.zshrc#L43-L49

My proposal instead of https://github.com/atweiden/fzf-extras/issues/12#issue-354748802 is to remove the branch number.

--- a/fzf-extras.zsh
+++ b/fzf-extras.zsh
@@ -242,11 +242,9 @@ fbr() {
       --format='%(refname:short)'
   )" || return

-  num_branches="$(echo $(wc -l <<< "$branches") | xargs echo -n)"
-
   branch="$(
     echo "$branches" \
-      | fzf-tmux -d "$((2 + $num_branches))" +m
+      | fzf-tmux +m
   )" || return

`fstash`

On fstash when I type Enter I get:

fstash:15: command not found: mapfile
h is not a valid reference

So instead I use this version https://github.com/benoittgt/fzf-extras/commit/0b86b15f6f8783ce77e933dd4ee5065696074c33

I open a PR on your repo if you want to review it, merge it if you like it and then update this PR. 🙂

aaaaahaaaaa commented 5 years ago

Changes have been merged, however there's still an issue with fstash: https://github.com/aaaaahaaaaa/fzf-extras/pull/1#issuecomment-426387674

deiga commented 5 years ago

@aaaaahaaaaa I can't reproduce any issue with fstash, what issue are you seeing?

teto commented 4 years ago

isn't emulate sh -c "source fzf-extras.sh" enough ? seems to work fine so far

atweiden / fzf-extras

Update fzf-extras.zsh with compatible bash scripts #13

12

`fbr`

`fstash`