Remove underscores on suggest - updated for master

[presidentbeef]

Sorry, I don't see how to properly update the previous pull request.

The only difference here is that 0e4b776 already took care of the unused variable.

Thanks.

[zenspider]

Again. No. We've been over this.

The current suggestion system is good.

[presidentbeef]

Sorry to be a bother, but I do not recall any discussion or rejection of this previously. Just for my own education, why is this not acceptable, given the performance and user experience improvements as shown in the pull request linked above?

[bowsersenior]

@drbrain closed issue #19 in favor of this issue without commenting on the merit of either one:

• Remove underscores and dashes when finding suggestions #19 (comment)

Then this issue was closed by @zenspider with a cryptic reference to a previous discussion.

It seems like this pull request would improve the suggestions to me. What is the downside?

In any case, it would be much better if there was more detail on why the pull request was originally closed. That way, other developers can understand rubygems better and make better pull requests in the future. I think it's very important for the developers of open-source projects to go the extra mile to provide thoughtful feedback, document references to previous issues, and suggest ways to improve a pull request. Especially with a project as important and popular as rubygems!

[practicingruby]

Folding on punctuation is pretty common in search/suggestion code, and this patch looks really simple to me. @zenspider: Can you give a specific reason why this should not be merged, or a reference to the old discussion?

/cc @bowsersenior @drbrain

[evanphx]

While this seems stale, it might be more likely to be pulled if it included tests.

[bowsersenior]

Seems like @sandal was cool with this without any tests since the change is so simple... Maybe one of the core committers could whip up a quick test and pull this thing in?

[presidentbeef]

I can add a test, maybe later today. It will essentially be the same as the current tests for suggestions.

[practicingruby]

@bowsersenior: That's not what my comment was meant to express. I was asking whether @zenspider had a specific reason for rejecting the patch, that's not to imply I thought the patch was ready to merge. That said, I think this is a good feature, so please add a test for it @presidentbeef.

[presidentbeef]

Today, I again wished this were accepted.

Without patch:

$ time gem install coffeescript
ERROR:  Could not find a valid gem 'coffeescript' (>= 0) in any repository
ERROR:  Possible alternatives: coffee-script, coffee-cup, coffeeshop, coffee_cup, applescript

real  0m10.095s
user  0m9.443s
sys   0m0.037s

With patch:

$ time gem install coffeescript
ERROR:  Could not find a valid gem 'coffeescript' (>= 0) in any repository
ERROR:  Possible alternatives: coffee-script

real  0m1.644s
user  0m0.982s
sys   0m0.032s

[luislavena]

@evanphx seems test has been included, however I don't like the assert of it, commented above.

[travisbot]

This pull request fails (merged 316ae3f into 294aa56).

[bowsersenior]

Thanks @presidentbeef for your continuing dedication to this pull request over the past year. I would have given up by now!

[luislavena]

@presidentbeef I'm testing this right now on Windows and will get back to you.

/cc @evanphx and @drbrain if no objection, will merge this to both master and 1.8 branch (since it is a performance regression on 1.8)

Thank you.

[zenspider]

This is not a performance regression.

[presidentbeef]

Just noticed this was merged. Thanks!