Provide "Did you mean? task_name" suggestions if task was not found #29
Conversation
| @@ -301,6 +321,33 @@ def get_description(task) | |||
| desc | |||
| end | |||
|
|
|||
| # Return a value representing the "cost" of transforming str1 into str2 | |||
| def levenshtein_distance(str1, str2) | |||
There was a problem hiding this comment.
We pass this method around, but I think it should be public in RubyGems, no?
There was a problem hiding this comment.
It is. We can just include Gem::Textand call #levenshtein_distance. I think you already know it had a bug, but maybe we shouldn't worry about it?
There was a problem hiding this comment.
On the other hand, Rake seems to keep a safe distance from RubyGems (maybe for a reason).
This could introduce some circular dependencies by depending on Gem itself.. and I'm not seeing anywhere besides testtask that uses it.
One could argue this method should be in a separate gem that both can require, and other people would find it useful. If that is the case, why not move it to core?
There was a problem hiding this comment.
Yeah, I've been thinking about it - providing levenshtein_distance as a stdlib of Ruby. Rubygems and Bundler already (and possibly Rake) use the Levenshtein algorithm so I think it's a good idea to move it to core. I'll file a feature request to https://bugs.ruby-lang.org.
That being said, I realized that rake still needs to have support for 1.8.7, meaning we can't just use fresh features introduced recently. Should we just not merge this for now? I don't feel comfortable forcing others to maintain code that is copy-and-pasted...
There was a problem hiding this comment.
We should avoid duplication whenever possible
There was a problem hiding this comment.
filed a ticket: https://bugs.ruby-lang.org/issues/10882
There was a problem hiding this comment.
I don't see why we can't merge this now while we wait for ruby to have levenshtein distance.
There was a problem hiding this comment.
If we merge this now I think we need to consider using Gem::Text instead of copy&paste
There was a problem hiding this comment.
The necessary methods for Gem::Text don't exist for all versions of RubyGems that rake supports.
|
closing this pull request as there's an ongoing discussion on https://bugs.ruby-lang.org/issues/11252. We might be able to use some of the features of |
similar to #19, but also suggests task names based on the edit distance.