Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Run fzf-lua with skim failed #1085

Closed
1 task
Spxg opened this issue Mar 12, 2024 · 8 comments
Closed
1 task

[BUG] Run fzf-lua with skim failed #1085

Spxg opened this issue Mar 12, 2024 · 8 comments

Comments

@Spxg
Copy link

Spxg commented Mar 12, 2024

Info

  • Operating System: macOS
  • Shell: zsh
  • Terminal: alacritty
  • nvim --version: NVIM v0.10.0-dev-2442+gfe74716cb
  • skim --version: sk 0.10.4
  • The issue is reproducible with mini.sh
fzf-lua configuration
require("fzf-lua").setup({ "skim" })

Description

The problems introduced by this commit feat: exclude icons from fuzzy matching. This --nth argument is not work for skim.

opts.fzf_opts["--nth"] = opts.fzf_opts["--nth"] or "-1.."

img_v3_028t_8ab689ff-e9e4-448c-8403-0ffd9efe93eg

@ibhagwan
Copy link
Owner

This --nth argument is not work for skim.

My bad, I didn’t test it with skim. I do recall it does support the flag so I’m confused why this doesn’t work, I’ll fix it.

@Spxg
Copy link
Author

Spxg commented Mar 12, 2024

This --nth argument is not work for skim.

My bad, I didn’t test it with skim. I do recall it does support the flag so I’m confused why this doesn’t work, I’ll fix it.

Actually this is a skim bug, I will fix it in my fork branch

@ibhagwan
Copy link
Owner

This --nth argument is not work for skim.

My bad, I didn’t test it with skim. I do recall it does support the flag so I’m confused why this doesn’t work, I’ll fix it.

Actually this is a skim bug, I will fix it in my fork branch

Nice, let me know when you fixed it and what went wrong?

In any event, I’ll make sure this works with a workaround as I can’t have this broken for everyone else but you.

@ibhagwan
Copy link
Owner

ibhagwan commented Mar 12, 2024

9a414ab

@Spxg, the latest commit works around the bug by using = for cmd flags (instead of just space), so instead of using --nth "-1.." we now use --nth="-1.." which works as expected.

@Spxg
Copy link
Author

Spxg commented Mar 12, 2024

9a414ab

@Spxg, the latest commit around the bug by using = for cmd flags (instead of just space), so instead of using --nth "-1.." we now use --nth="-1.." which works as expected.

I fixed this "with space" problem and upgraded to clap 4 to solve some wired problems. skim-rs/skim#562
The author seems to have been inactive for a long time, so it seems useless to submit a PR.

@Spxg
Copy link
Author

Spxg commented Mar 12, 2024

This --nth argument is not work for skim.

My bad, I didn’t test it with skim. I do recall it does support the flag so I’m confused why this doesn’t work, I’ll fix it.

Actually this is a skim bug, I will fix it in my fork branch

Nice, let me know when you fixed it and what went wrong?

In any event, I’ll make sure this works with a workaround as I can’t have this broken for everyone else but you.

image

Looking at the code, I found that the author actually handles negative numbers, but the - sign will be specially processed by clap (Command Line Argument Parser for Rust). Fortunately clap provides a flag to control this behavior: https://docs.rs/clap/latest/clap/struct.Arg.html#method.allow_hyphen_values.

@ibhagwan
Copy link
Owner

This --nth argument is not work for skim.

My bad, I didn’t test it with skim. I do recall it does support the flag so I’m confused why this doesn’t work, I’ll fix it.

Actually this is a skim bug, I will fix it in my fork branch

Nice, let me know when you fixed it and what went wrong?
In any event, I’ll make sure this works with a workaround as I can’t have this broken for everyone else but you.

image Looking at the code, I found that the author actually handles negative numbers, but the `-` sign will be specially processed by clap (Command Line Argument Parser for Rust). Fortunately clap provides a flag to control this behavior: https://docs.rs/clap/latest/clap/struct.Arg.html#method.allow_hyphen_values.

Very nice! I recently also used clap but had no idea about this behavior.

Btw, have you see this fork from skim-rs/skim#561 claiming to have better performance than fzf?

If this is true I would’ve loved for this to be PR’d and merged.

@Spxg
Copy link
Author

Spxg commented Mar 14, 2024

This --nth argument is not work for skim.

My bad, I didn’t test it with skim. I do recall it does support the flag so I’m confused why this doesn’t work, I’ll fix it.

Actually this is a skim bug, I will fix it in my fork branch

Nice, let me know when you fixed it and what went wrong?
In any event, I’ll make sure this works with a workaround as I can’t have this broken for everyone else but you.

image Looking at the code, I found that the author actually handles negative numbers, but the `-` sign will be specially processed by clap (Command Line Argument Parser for Rust). Fortunately clap provides a flag to control this behavior: https://docs.rs/clap/latest/clap/struct.Arg.html#method.allow_hyphen_values.

Very nice! I recently also used clap but had no idea about this behavior.

Btw, have you see this fork from lotabout/skim#561 claiming to have better performance than fzf?

If this is true I would’ve loved for this to be PR’d and merged.

I don't know much about optimizing performance, but I think these optimizations are effective.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants