Skip to content

feat: fish autocomplete #52

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: fish autocomplete #52

wants to merge 6 commits into from

Conversation

loks0n
Copy link

@loks0n loks0n commented Feb 11, 2025

Adds auto completion for fish shell

@loks0n loks0n requested a review from a team as a code owner February 11, 2025 10:43
@loks0n
Copy link
Author

loks0n commented Feb 11, 2025

@molisani Love the package! Let me know if you have any suggestions

@molisani
Copy link
Member

Ah, thank you very much for this contribution! This looks like all the machinery needed to register an autocomplete command with fish.

However, I'm not familiar with how fish autocomplete works, so do you know if the bash autocomplete format/strategy is compatible with fish? If not, we probably need something like this but for fish so that it can format the proposals correctly. (Although looking at that now, we could move that logic into some part of the @stricli/auto-complete module)

I don't think we would necessarily need that to get this PR merged, as the logic you've written up here is still correct and necessary, but I would want to make sure that we have a through-line for confirming that auto-complete works as expected.

Signed-off-by: loks0n <[email protected]>
.
Signed-off-by: loks0n <[email protected]>
@loks0n
Copy link
Author

loks0n commented Feb 19, 2025

However, I'm not familiar with how fish autocomplete works, so do you know if the bash autocomplete format/strategy is compatible with fish? If not, we probably need something like this but for fish so that it can format the proposals correctly. (Although looking at that now, we could move that logic into some part of the @stricli/auto-complete module)

Ah right! I think it's slightly different. I've gone ahead and created a new file specifically for fish. Let me know if you want to go with an alternative approach.

@molisani
Copy link
Member

molisani commented Mar 6, 2025

I've created #58 which changes the approach for handling different shells. It needs further review from my team and there may be some small changes before it gets merged, but I think it would be easier to land your changes on top of #58. I'm currently out-of-office this week but I think once I am back I can prioritize that PR to make way for your feature.

@loks0n
Copy link
Author

loks0n commented Mar 12, 2025

I've created #58 which changes the approach for handling different shells. It needs further review from my team and there may be some small changes before it gets merged, but I think it would be easier to land your changes on top of #58.

OK, no worries, I'll rebase once that is merged 👍

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

Successfully merging this pull request may close these issues.

2 participants