Aloxaf / fzf-tab

Replace zsh's default completion selection menu with fzf!
MIT License
2.98k stars 90 forks source link

feat: prefix completion #413

Closed Aloxaf closed 4 months ago

Aloxaf commented 4 months ago

This PR is another approch to let fzf-tab complete the common prefix. It can handle some corner cases like https://github.com/Aloxaf/fzf-tab/pull/384#issuecomment-1957202220 and zsh-z.

how to enable

# temporary
zstyle ':completion:*' menu no

known bugs

It breaks multiple selections

related issues/pr

8 #384

cohml commented 4 months ago

Coming here from https://github.com/Aloxaf/fzf-tab/pull/384#issuecomment-1962422007.

Not sure if testing this requires rebuilding the fzf-tab module, but when I tried to, I got an error:

❯ source fzf-tab.zsh
❯ which build-fzf-tab-module
build-fzf-tab-module () {
    -ftb-build-module $@
}
❯ build-fzf-tab-module
cd . && ./.preconfig
./.preconfig: line 5: autoconf: command not found
./Util/preconfig: ./.preconfig failed (status 127)
-ftb-build-module:30: no such file or directory: ./configure
make: *** No targets specified and no makefile found.  Stop.
The module building has failed. See the output above for details.

I don't understand the error, but after getting it my $PWD was ./fzf-tab/modules/zsh/5.9. JFYI.

Anyway I continued to test, in the event that rebuilding wasn't necessary. All tests were completed on the latest MacOS with zsh 5.9.

I checked all edge cases discussed so far on #384. All appear to have been resolved. Nice work!

However one thing did stand out, which was perhaps intentional but personally I'm not sure I like it...

Consider this tree:

❯ TMPDIR=$(mktemp -d) && cd $TMPDIR
❯ mkdir foo bar
❯ touch foo/ba{r,z}
❯ tree -F
./
├── bar/
└── foo/
    ├── bar
    └── baz

3 directories, 2 files

If I do this

> ls <TAB>

fzf opens with bar/ and foo/, as expected. But if I arrow down to foo/ and press /, fzf exits and my prompt immediately completes to

> ls foo/ba

While I guess I understand how this behavior would fit into the new "complete longest prefix" paradigm, it was not expected. In particular I didn't expect it to terminate fzf, because on main it doesn't do so (IIRC).

Curious how others feel about this though. I suppose I could be swayed either way, but it was definitely surprising at first.

cohml commented 4 months ago

if I arrow down to foo/ and press /, fzf exits and my prompt immediately completes to

> ls foo/ba

And actually, I just tested a slightly different scenario, but the results seem inconsistent:

❯ mkdir foo bar
❯ touch foo/.ba{r,z} # note these are now "hidden" files
❯ tree -Fa
./
├── bar/
└── foo/
    ├── .bar
    └── .baz

3 directories, 2 files

When I enter

❯ ls <TAB>

and select foo/ with the / key, my prompt completes to just ls foo/, not ls foo/.ba.

I suppose it's possible this is due to some zsh configuration about hidden files that's particular to me. But I thought I'd bring it up here for discussion.

Aloxaf commented 4 months ago

I don't understand the error, but after getting it my $PWD was ./fzf-tab/modules/zsh/5.9. JFYI.

This error is caused by the missing of autoconf.

Not restoring pwd is a bug, fixed in https://github.com/Aloxaf/fzf-tab/commit/cdc34ee6469581a1ea49a8f8b6bb5395a5e98fe7

While I guess I understand how this behavior would fit into the new "complete longest prefix" paradigm, it was not expected. In particular I didn't expect it to terminate fzf, because on main it doesn't do so (IIRC).

Curious how others feel about this though. I suppose I could be swayed either way, but it was definitely surprising at first.

I agree with you. It breaks the coherence of the continuous completion. fixed in https://github.com/Aloxaf/fzf-tab/pull/413/commits/7b1e083f112d3741ae7cbdc166ca98f50edf4b82

I suppose it's possible this is due to some zsh configuration about hidden files that's particular to me. But I thought I'd bring it up here for discussion.

Yes, this is zsh's default behaviour. See https://man.archlinux.org/man/zshoptions.1.en#GLOB_DOTS

cohml commented 4 months ago

On the latest commit: After recreating the same TMPDIR tree shown above, scrolling to foo, and pressing /, fzf doesn't exit which is good. But I now see this, which is not correct:

image

It seems to include all the previous and new completions altogether. foo/foo/ is not a path that exists, for example. The results post-/ should only include bar and baz.

Perhaps you need to flush the buffer or something so that the options shown before pressing / (i.e., foo/ and bar/) are purged from the list of results?

Aloxaf commented 4 months ago

@cohml This is indeed a minor bug, thank you for pointing it out!

cohml commented 4 months ago

So everything reported thus far seems to be fixed and working great 👍

But one bizarre thing is happening now. Again I'm unable to figure out if it's just me vs. reproducible by others. But I'll report my observations anyway, just in case.

First, note that my local branches are all up to date with the remote:

❯ for branch in master feat/complete-prefix; do git checkout $branch && git pull; done
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
Already up to date.
Switched to branch 'feat/complete-prefix'
Your branch is up to date with 'origin/feat/complete-prefix'.
Already up to date.

When I check out master and restart my shell, nothing special happens. This is good and expected.

But when I check out feat/complete-prefix and restart my shell, the following error now appears upon startup:

_zsh_highlight_bind_widgets:zle:46: bad option: -t

[!IMPORTANT] So the only thing that changes between me getting that error and not getting that error is my local fzf-tab branch.

This confuses me, because that widget comes from the zsh-syntax-highlighting plugin, not fzf-tab ...

❯ git -C zsh-syntax-highlighting grep _zsh_highlight_bind_widgets
zsh-syntax-highlighting.zsh:# Helper for _zsh_highlight_bind_widgets
zsh-syntax-highlighting.zsh:  _zsh_highlight_bind_widgets(){}
zsh-syntax-highlighting.zsh:  _zsh_highlight_bind_widgets()
zsh-syntax-highlighting.zsh:_zsh_highlight_bind_widgets || {

... and because the fzf-tab source code doesn't even seem to invoke it at all:

❯ git -C fzf-tab grep -q _zsh_highlight_bind_widgets && echo matches || echo no matches
no matches

Can you speculate what's happening here? Is this a problem with the feat/complete-prefix branch?

[!NOTE]

  1. My zsh-syntax-highlighting clone is up to date:
    ❯ git -C zsh-syntax-highlighting pull
    Already up to date.
  2. I source fzf-tab before I source zsh-syntax-highlighting. The errors disappears if sourced in the other direction.
  3. I use both MacOS and Amazon Linux. This issue only seems to happen on the latter.

Apologies if this is a wild goose chase. This one niggle aside, your changes are fantastic :)

Aloxaf commented 4 months ago

@cohml I think this is because I created a widget called -ftb-complete-apply. I rename it to _fzf-tab-apply, please update and try again.

cohml commented 4 months ago

Yup, that seems to fix it!

I am now out of ideas for further testing. This feature seems to work great - Cheers!

All that seems missing is some documentation for it.

pablospe commented 4 months ago

How to enable this? Or, is the default behavior now? Not sure from the documentation, in the PR description says:

# temporary
zstyle ':completion:*' menu no
Aloxaf commented 4 months ago

@pablospe

How to enable this?

zstyle ':completion:*' menu no

cohml commented 4 months ago

That is strange. With my master branch up to date I get this new prefix completion behavior, yet I do not source that line anywhere on startup.

@Aloxaf Are you sure that line is necessary?

Aloxaf commented 4 months ago

That is strange. With my master branch up to date I get this new prefix completion behavior, yet I do not source that line anywhere on startup.

@Aloxaf Are you sure that line is necessary?

It depends on your configuration.

If you have configured something like setopt menu_complete or zstyle ':completion:*' menu yes select search, then this line can override the code above; otherwise, you can omit it.

cohml commented 4 months ago

I see. That may be it, but I don't see anything like that in my .zshrc (or more accurately, the file my .zshrc sources which configures fzf-tab). Then again, my knowledge of zstyle is quite incomplete. For reference, here is the file.

#!/bin/zsh

# default fzf settings to apply globally
FZF_TAB_DEFAULT_FZF_FLAGS=(
  "--height=~95%"
  "--multi" # TODO: still not working for some reason
  "--no-exact"
)
zstyle ":fzf-tab:*" fzf-flags "${FZF_TAB_DEFAULT_FZF_FLAGS[@]}"

# when completing environment variables, show their values (wrapped)
# in the preview window ("<unset>" if no value/empty string)
zstyle ":fzf-tab:complete:(-command-|-parameter-|-brace-parameter-|export|unset|expand):*" fzf-flags "--preview-window=wrap" "${FZF_TAB_DEFAULT_FZF_FLAGS[@]}"
zstyle ":fzf-tab:complete:(-command-|-parameter-|-brace-parameter-|export|unset|expand):*" fzf-preview "[[ -n \${(P)word} ]] && echo \${(P)word} || echo \<unset\>"

# don't sort git checkout output
zstyle ":completion:*:git-checkout:*" sort false

# use $ZLS_COLORS to color filenames by type
zstyle ":completion:*" list-colors "${(s.:.)ZLS_COLORS}"

# set descriptions format to enable options to be grouped into "completion groups"
zstyle ":completion:*:descriptions" format "[%d]"

 # switch between "completion groups" using ";" and "," keys
FZF_TAB_SWITCH_GROUP_FORWARD_CHAR=";"
FZF_TAB_SWITCH_GROUP_BACKWARD_CHAR=","
zstyle ":fzf-tab:*" switch-group "${FZF_TAB_SWITCH_GROUP_BACKWARD_CHAR}" "${FZF_TAB_SWITCH_GROUP_FORWARD_CHAR}"

# set prefix to prepend to each option (empty string/disabled in this case)
FZF_TAB_OPTIONS_PREFIX_STRING=
zstyle ":fzf-tab:*" prefix "${FZF_TAB_OPTIONS_PREFIX_STRING}"

# clean up namespace
unset FZF_TAB_DEFAULT_FZF_FLAGS \
      FZF_TAB_SWITCH_GROUP_BACKWARD_CHAR \
      FZF_TAB_OPTIONS_PREFIX_STRING \
      FZF_TAB_SWITCH_GROUP_FORWARD_STRING

I'm happy to move this to a dedicated issue or discussion if you feel we're straying too far from the original PR.

zydou commented 4 months ago

Hello dev, I found a bug related in this PR. Please pay attention to the word capitalization in the following example.

Previously:

$ ls -1
AppName
appname-2
appname-3

Type ls app<TAB>:

$ ls app
> app█
  3/3 (0)
> AppName
  appname-1
  appname-2

After this PR is merged.

$ ls -1
AppName
appname-2
appname-3

type ls app<TAB> gives ls appName, then type <TAB> again:

$ ls appName
> appName█
  0/3 (0)
Aloxaf commented 4 months ago

@zydou You can add -i to fzf-flags.

zydou commented 4 months ago

@Aloxaf Could you elaborate a little more?

I added zstyle ':fzf-tab:*' fzf-flags '-i' to my .zshrc, but nothing changed.

LinoWhy commented 3 months ago

Hello dev, I found a bug related in this PR. Please pay attention to the word capitalization in the following example.

Previously:

$ ls -1
AppName
appname-2
appname-3

Type ls app<TAB>:

$ ls app
> app█
  3/3 (0)
> AppName
  appname-1
  appname-2

After this PR is merged.

$ ls -1
AppName
appname-2
appname-3

type ls app<TAB> gives ls appName, then type <TAB> again:

$ ls appName
> appName█
  0/3 (0)

Same issue here.

zydou commented 3 months ago

@LinoWhy Hi, I just pinned the version to 02ce77cc855db31fabe0f98181078f3ced0b7b70. This is the last commit that works with the same word in different cases.

pablospe commented 3 months ago

It might be beneficial to create a new Issue for this. And by mentioning it here, you can ensure it doesn't get overlooked.

LinoWhy commented 3 months ago

@zydou Set fzf option as below, the default is smart case. export FZF_DEFAULT_OPTS="-i"

@zydou You can add -i to fzf-flags.

zydou commented 3 months ago

@LinoWhy Thanks a lot! I can confirm export FZF_DEFAULT_OPTS="-i" works for the latest commit.