-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Emit suggested generator names when not found #15497
Emit suggested generator names when not found #15497
Conversation
👍 |
One PR at a time, making Rails more beginner and derp-friendly. 👍 |
Nice! Developer happiness all the way! |
@@ -21,6 +21,8 @@ module Generators | |||
autoload :ResourceHelpers, 'rails/generators/resource_helpers' | |||
autoload :TestCase, 'rails/generators/test_case' | |||
|
|||
extend Gem::Text |
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.
I think is better to not couple the generator with RubyGems. Can we just copy the implementation?
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.
It is already in the guides https://github.com/rails/rails/blob/master/guides/rails_guides/levenshtein.rb but I do not think it is usable by railties in that location. Is it okay to duplicate this code or should we put it somewhere it can be used by both?
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.
It is ok for duplicate the code.
I updated to duplicate the code instead of depending on rubygems. I've rebased in the fix from master so hopefully tests will pass now.I updated to duplicate the code instead of depending on rubygems. I've rebased in the fix from master so hopefully tests will pass now. |
Great! Mind to add a CHANGELOG entry? |
When someone types in a generator command it currently outputs all generators. Instead we can attempt to find a subtle mis-spelling by running all generator names through a levenshtein_distance algorithm provided by rubygems. So now a failure looks like this: ```ruby $ rails generate migratioooons Could not find generator 'migratioooons'. Maybe you meant 'migration' or 'integration_test' or 'generator' Run `rails generate --help` for more options. ``` If the suggestions are bad we leave the user with the hint to run `rails generate --help` to see all commands.
done |
…r-failure-messages Emit suggested generator names when not found
✨ ✨ ✨ ✨ ✨ Awesome |
assert_match "`rails generate --help`", output | ||
end | ||
|
||
def test_generator_suggestions |
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.
@schneems I think we should add a test case for multiple suggestions as well.
end | ||
|
||
protected | ||
|
||
# This code is based directly on the Text gem implementation | ||
# Returns a value representing the "cost" of transforming str1 into str2 | ||
def self.levenshtein_distance str1, str2 |
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.
Great, but omg why is this algorithm hiding in the Rails generator?
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.
It is only used here, I don't see why it should be somewhere else.
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.
Is that what ActiveSupport is for? Anything not part of the app, models, views, controllers? I'd make a PR.
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.
Active support is only for reusable piece of code in the framework. This
code is only used here and we don't plan to reuse neither to make it
available for users. That said, thank you for your suggestion but there is
no need for a pull request.
On Tue, Dec 30, 2014, 19:23 Benjamin Fleischer notifications@github.com
wrote:
In railties/lib/rails/generators.rb
#15497 (diff):end protected
# This code is based directly on the Text gem implementation
# Returns a value representing the "cost" of transforming str1 into str2
def self.levenshtein_distance str1, str2
Is that what ActiveSupport is for? Anything not part of the app, models,
views, controllers? I'd make a PR.—
Reply to this email directly or view it on GitHub
https://github.com/rails/rails/pull/15497/files#r22365394.
When someone types in a generator command it currently outputs all generators. Instead we can attempt to find a subtle mis-spelling by running all generator names through a
levenshtein_distance
algorithm provided by rubygems (Gem::Text
).So now a failure looks like this:
If the suggestions are bad we leave the user with the hint to run
rails generate --help
to see all commands.