-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[PasswordHasher] Add autocompletion for security commands #43653
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
Conversation
wouterj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was quick, thanks!
I have a question regarding @wouterj 's comment on the issue
Ignore my comment, I didn't know about this $userClasses property.
Also, the password is the first argument right now, should we swap it to be after user-class?
No, it's fine like this (autocompletion should support it this way).
Can you please also add tests? (see the other completion PRs for examples)
src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/PasswordHasher/Command/UserPasswordHashCommand.php
Outdated
Show resolved
Hide resolved
ca1e869 to
534c766
Compare
src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php
Outdated
Show resolved
Hide resolved
534c766 to
b8addda
Compare
src/Symfony/Component/PasswordHasher/Command/UserPasswordHashCommand.php
Show resolved
Hide resolved
b8addda to
5c0f762
Compare
5c0f762 to
49f45a9
Compare
wouterj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, let's block this for a bit as we first need to handle the backslashes properly in the completion feature: #43598
chalasr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlocked
|
Thank you @94noni. |
|
Tests are failing on 5.4: https://github.com/symfony/symfony/runs/4055700364?check_suite_focus=true#step:8:2615 |
This PR was merged into the 5.4 branch. Discussion ---------- [PasswordHasher] Fix completion tests | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #43653 (comment) | License | MIT | Doc PR | N/A Commits ------- fde1b0c [PasswordHasher] Fix completion tests
Related to #43594 (comment)
I have a question regarding @wouterj 's comment on the issue
Also, the
passwordis the first argument right now, should we swap it to be afteruser-class?Still WIP, I am using
fishand want to test as well #43641