-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Use DidYouMean::SpellChecker for gem suggestions in Bundler #3857
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
| return words[0] | ||
| end | ||
|
|
||
| [words[0..-2].join(", "), words[-1]].join(" or ") |
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.
| [words[0..-2].join(", "), words[-1]].join(" or ") | |
| [words[0..-2].join(", "), words[-1]].join(", or ") |
Personally I like oxford commas ("apple, orange, or pear"). I went without it so the output is similar to how it has always been ("apple, orange or pear").
| # frozen_string_literal: true | ||
|
|
||
| # This module is not used anywhere in Bundler | ||
| # It is included for backwards compatibility in-case someone is relying on it |
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 left this module in here so I don't pull the rug out from someone who might be using it in their own project. I would like to delete it, is there a deprecation path for stuff like this?
austinpray
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.
Same concern as #3856 (review)
c2d83ae to
aaaedc8
Compare
|
@austinpray Can you update this Pr by |
aaaedc8 to
990970f
Compare
9a5c3d1 to
6a67d88
Compare
replaces Bundler::SimilarityDetector with DidYouMean::SpellChecker
6a67d88 to
d604c1d
Compare
https://github.com/ruby/rubygems/actions/runs/19458155903/job/55676075439?pr=3857 ``` -Did you mean 'methods' or 'method'? +Could not find gem 'methosd'. +Did you mean 'method' or 'methods'? ```
|
@austinpray Sorry to my late action. I'm happy to introduce this feature at Bundler 4 and Ruby 4.0. Thank you! |
DidYouMean::Levenshtein.distancefromdid_you_mean-1.4.0#3856Description:
replaces
Bundler::SimilarityDetectorwithDidYouMean::SpellCheckerBundler::SimilarityDetector.levenshtein_distancedynamic programming implementation could be optimized quite a bit.DidYouMean::SpellCheckeris highly optimized.RubyGems 3.0 supports Ruby 2.3 or later. Ruby 2.3 and later ships with the
did_you_meangem.https://stdgems.org/did_you_mean/
Higher quality spelling suggestions
The algorithm seems to
Tested with the rails 6 gemfile. Here's some results from manually stressing the spell checker:
Benchmark
💹 Benchmark code (permalink)
This looks like a 7x performance boost. The
DidYouMeanimplementation also uses less memory and such.Notes for reviewer
Bundler::SimilarityDetectoris not used in Bundler anymore. I would like to delete it, but I haven't haven't because I am not sure if we should pull the rug out from people that might be using for their own purposes. What should we do to phase out this module?What was the end-user or developer problem that led to this PR?
Was improving performance over in #3856 and saw this as an opportunity as well.
What is your fix for the problem, implemented in this PR?
Use
DidYouMean::SpellCheckeras a drop-in replacement forBundler::SimilarityDetectorTasks:
I will abide by the code of conduct.