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

[FrameworkBundle][DX] Add Levenshtein suggesters to AbstractConfigCommand #17484

Merged
merged 1 commit into from
Mar 2, 2016

Conversation

kix
Copy link
Contributor

@kix kix commented Jan 21, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR none

It could be helpful to output the best guesses for bundle names and container extension aliases when one could not be found by the exact query.

Perhaps, I could regroup the logic so that it only looks through bundle names if the Bundle suffix is present, but I guess this might narrow the use case scope here.

@kix kix force-pushed the use-levenshtein-for-findextension branch from a031489 to 3c0b0ae Compare January 21, 2016 23:04
@fabpot
Copy link
Member

fabpot commented Jan 23, 2016

I'm not sure about this one. Extension names are derived from bundle names.

@kix
Copy link
Contributor Author

kix commented Jan 23, 2016

@fabpot, well, technically they should be derived (they do match bundle names right after the bundle has been generated), but there's nothing that could stop someone from (mistakenly) overriding those. That's going against the rules, yes, but I think it's better to handle these cases transparently.
Also, checking Levenshtein distance for both the bundle name and its extension alias seems to make sense in both cases because of this:

levenshtein('SensioFrameworkExtraBundle', 'sensioframeworkextra') => int(9)
levenshtein('sensio_framework_extra', 'sensioframeworkextra')) => int(2)

I mean, the reason here is just suggesting the best possible match for the given input, and I think this solution might be satisfying.

@fabpot
Copy link
Member

fabpot commented Jan 25, 2016

Fair enough, can you add some tests?

@kix
Copy link
Contributor Author

kix commented Jan 25, 2016

@fabpot, sure, will do.

@javiereguiluz javiereguiluz added FrameworkBundle DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Jan 26, 2016
@fabpot
Copy link
Member

fabpot commented Feb 26, 2016

@kix any news?

@kix
Copy link
Contributor Author

kix commented Feb 26, 2016

@fabpot, sorry, not yet :(

@fabpot
Copy link
Member

fabpot commented Mar 2, 2016

Thank you @kix.

@fabpot fabpot merged commit 3c0b0ae into symfony:master Mar 2, 2016
fabpot added a commit that referenced this pull request Mar 2, 2016
…stractConfigCommand (kix)

This PR was merged into the 3.1-dev branch.

Discussion
----------

[FrameworkBundle][DX] Add Levenshtein suggesters to AbstractConfigCommand

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | none

It could be helpful to output the best guesses for bundle names and container extension aliases when one could not be found by the exact query.

Perhaps, I could regroup the logic so that it only looks through bundle names if the `Bundle` suffix is present, but I guess this might narrow the use case scope here.

Commits
-------

3c0b0ae Add Levenshtein suggesters to AbstractConfigCommand
@fabpot
Copy link
Member

fabpot commented Mar 2, 2016

Writing tests is really cumbersome as we would have to mock half of Symfony :) So, I've merged as is.

@kix
Copy link
Contributor Author

kix commented Mar 2, 2016

@fabpot, thanks for taking a look, I didn't really have any idea on how do I test this :)

@kix kix deleted the use-levenshtein-for-findextension branch March 2, 2016 14:36
@fabpot fabpot mentioned this pull request May 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) FrameworkBundle Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants