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

[bash] bring fzf’s own bash completion up to date #3471

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

calestyo
Copy link
Contributor

@calestyo calestyo commented Oct 9, 2023

Well, I tried to find out all possible options and "fixed" values as good as I could from the actual parsing code.

Unfortunately both, --help-output and the manpage seem to be also outdated (see #3470), so these were couldn’t really be used as a reference.

Yeah,... I remember your comment in #3457 (comment) ... but it sounded rather unclear whether/when you'd go that "cobra"-way... and I don't want fzf’s own completion to be dropped.

So here's an update.

That being said, I still think we should do #3457, but maybe we can discuss that in terms of some larger ideas.

btw:

Why are you doing this:

fzf/shell/completion.bash

Lines 123 to 126 in a0d61b4

--history)
COMPREPLY=()
return 0
;;

?
Can it be dropped?

This orders and groups completed options and values in just as they appear in
the code respectively, for some option values, as they’d be printed in the
`--help`-output.

It does not add support for completion of `:` right after values that support an
optional `:some-further-value` postfix.
Neither does it add support for the `--option=value`-style.

Signed-off-by: Christoph Anton Mitterer <[email protected]>
@junegunn
Copy link
Owner

Thank you.

it sounded rather unclear whether/when you'd go that "cobra"-way...

Completion for fzf itself is not my priority right now so it's not clear when I'm going to work on it. But what is clear is that I don't want to maintain three different implementations to provide the same functionality across bash, zsh, and fish. So yeah, Cobra (or any equivalent library) is the way to go.

btw: Why are you doing this:

fzf/shell/completion.bash

Lines 123 to 126 in a0d61b4

--history)
COMPREPLY=()
return 0
;;

?
Can it be dropped?

I don't really remember. If it doesn't make any difference, we can drop it.

Presumably, the dropped code is not needed for any effect, thus drop it.

Signed-off-by: Christoph Anton Mitterer <[email protected]>
@calestyo
Copy link
Contributor Author

I don't really remember. If it doesn't make any difference, we can drop it.

Uhm... well at least I can't see any difference. Though I'm not really a completion expert and there may be some side-effects that I just don't realise.

It seems to have been added in 3b52811, but AFAICS there is no information on why the array is set to an empty one.

I've added a commit on top that drops the part, and if it actually breaks something,... will blame everything on you. 😜😇

@junegunn junegunn merged commit 4feaf31 into junegunn:master Oct 11, 2023
5 checks passed
@calestyo calestyo deleted the improve-fzf-utility-completion branch October 11, 2023 02:20
@calestyo
Copy link
Contributor Author

btw: Not a serious problem, but any reason for always squashing the commits? Makes cleaning up the local branches more work ^^

@junegunn
Copy link
Owner

I squash when the intermediate changes are minor and don't provide much value to be seen separately.

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

Successfully merging this pull request may close these issues.

None yet

2 participants