Skip to content

[Console] Issue 43602 : Add fish completion #43641

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 13, 2021
Merged

[Console] Issue 43602 : Add fish completion #43641

merged 1 commit into from
Dec 13, 2021

Conversation

guillaume-a
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? no
New feature? no
Deprecations? no
Tickets Fix #43602
License MIT

I tried to implement fish auto completion.
I hope it will work fine for you guys, let me know.

You can execute bin/console completion fish >> ~/.config/fish/completions/sf_console.fish and source the file

@guillaume-a guillaume-a requested a review from chalasr as a code owner October 21, 2021 21:15
@carsonbot carsonbot added this to the 5.4 milestone Oct 21, 2021
@guillaume-a
Copy link
Contributor Author

I just saw test errors. I'll fix it tomorrow.

@carsonbot carsonbot changed the title Issue 43602 : Add fish completion [Console] Issue 43602 : Add fish completion Oct 21, 2021
@94noni 94noni mentioned this pull request Oct 22, 2021
Merged
@carsonbot

This comment has been minimized.

Sign in to view
@stof
Copy link
Member

stof commented Oct 22, 2021

Does this handle properly values containing backslashes or spaces ? see #43664 for the bash completion

@guillaume-a
Copy link
Contributor Author

@stof otb I don't think. i'll take a look soon. Thanks for the feedback.

@fabpot fabpot modified the milestones: 5.4, 6.1 Oct 29, 2021
@guillaume-a
Copy link
Contributor Author

@fabpot fabpot added this to the 6.1 milestone 14 minutes ago

Does this mean i have to change my rebase branch for this PR ?

@guillaume-a guillaume-a force-pushed the issue-43602 branch 2 times, most recently from 4095d09 to e1327df Compare October 29, 2021 13:40
fabpot added a commit that referenced this pull request Oct 29, 2021
@fabpot
feature #43653 [PasswordHasher] Add autocompletion for security comma…
c3522c4 
…nds (noniagriconomie)

This PR was merged into the 5.4 branch.

Discussion
----------

[PasswordHasher] Add autocompletion for security commands

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Ref #43594
| License       | MIT
| Doc PR        |

Related to #43594 (comment)
I have a question regarding `@wouterj` 's comment on the issue
Also, the `password` is the first argument right now, should we swap it to be after `user-class`?

Still WIP, I am using `fish` and want to test as well #43641

Commits
-------

49f45a9 [Console] Add autocompletion for security commands
@alexislefebvre
Copy link
Contributor

If it can help, there was a fish plugin for Symfony 2: https://github.com/mojoLyon/fish-sf

@guillaume-a
Copy link
Contributor Author

Rebased onto 6.1, and seams to work. At least "on my machine" ;)

@GromNaN
Copy link
Member

GromNaN commented Dec 9, 2021

Does it work with the following features:

@guillaume-a
Copy link
Contributor Author

#44461 it seems to work OTB with fish.
image

#43664 : And this too
image

wouterj
wouterj requested changes Dec 11, 2021
View reviewed changes
Copy link
Member

@wouterj wouterj left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested this and it indeed works quite nicely! Thanks for working on this @guillaume-a!

I only discovered it doesn't work correctly when autocompleting something in the middle of a command, e.g. bin/console secret:set --| SOME_SECRET (| being the cursor) should provide completion values for -- option names.

@guillaume-a guillaume-a force-pushed the issue-43602 branch 2 times, most recently from bff8096 to 5335026 Compare December 13, 2021 14:50
wouterj
wouterj approved these changes Dec 13, 2021
View reviewed changes
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

One thing we should probably update is how we document the installation process (as it's different than Bash). But that'll break a lot of test, let's rethink how we use the help message outside of this PR.

@fabpot
Copy link
Member

fabpot commented Dec 13, 2021

Thank you @guillaume-a.

@fabpot fabpot mentioned this pull request Dec 13, 2021
Closed
@fabpot fabpot merged commit 64a4f02 into symfony:6.1 Dec 13, 2021
@fabpot fabpot mentioned this pull request Apr 15, 2022
Merged
@GromNaN GromNaN mentioned this pull request Apr 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkarlovi dkarlovi dkarlovi requested changes

@fabpot fabpot fabpot approved these changes

@wouterj wouterj wouterj approved these changes

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

Assignees
No one assigned
Labels
Console Feature Status: Reviewed
Projects
None yet
Milestone
6.1
Development

Successfully merging this pull request may close these issues.

[Console] Add completion for fish
9 participants
@guillaume-a @carsonbot @stof @alexislefebvre @GromNaN @fabpot @dkarlovi @wouterj @derrabus