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

Implement KeyError checker #71

Closed
wants to merge 2 commits into from
Closed

Implement KeyError checker #71

wants to merge 2 commits into from

Conversation

ksss
Copy link
Contributor

@ksss ksss commented Feb 5, 2016

I try experimental implementation of catch the KeyError.

hash = {foo: 1, bar: 2, baz: 3}
hash.fetch(:fooo)
# KeyError: key not found: :fooo
# Did you mean?  :foo

module Correctable
prepend_features KeyError
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the following over #prepend_features:

KeyError.prepend DidYouMean::Correctable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds cool

@ksss
Copy link
Contributor Author

ksss commented Feb 6, 2016

Thank you for your kind reply.
I fixed code and rebased commit.

end
Hash.prepend KeyErrorReciver

class KeyNameChecker < MethodNameChecker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to let KeyNameChecker inherit from MethodNameChecker since KeyError doesn't behave like NameError. It can just include SpellCheckable and implement what is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misunderstood about "abstracted away".
I'll implement KeyNameChecker again.

@ksss
Copy link
Contributor Author

ksss commented Feb 8, 2016

I rebased again.
How about this?

klass.send(:attr, :keys, :name)
end
end
KeyError.prepend KeyErrorWithNameAndKeys
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need a module then.

KeyError.send(:attr, :name, :keys)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.

@ksss
Copy link
Contributor Author

ksss commented Feb 9, 2016

I have a question.

Can we remove this library backtrace?

test.rb

#! /usr/bin/env ruby

require 'did_you_mean'
require 'did_you_mean/extra_features'

{foo: 1}.fetch(:ffo)

expect

$ bundle ex ruby test.rb
test.rb:6:in `<main>': `fetch': key not found: :ffo (KeyError)
Did you mean?  :foo

actual

$ bundle ex ruby test.rb
*/did_you_mean/lib/did_you_mean/extra_features/key_error.rb:5:in `fetch': key not found: :ffo (KeyError)
Did you mean?  :foo
    from */did_you_mean/lib/did_you_mean/extra_features/key_error.rb:5:in `fetch'
    from test.rb:6:in `<main>'

@kunitoo
Copy link

kunitoo commented Feb 11, 2016

I think really good pull request.
I expect to similar behavior also for Hash#fetch_values and ENV.fetch.
What do you think?

test.rb

#! /usr/bin/env ruby

require 'did_you_mean'
require 'did_you_mean/extra_features'

{foo: 1}.fetch_values(:ffo)

expect

$ bundle exec ruby test.rb
test.rb:6:in `fetch_values': key not found: :ffo (KeyError)
Did you mean?  :foo

actual

$ bundle exec ruby test.rb
`fetch_values': key not found: :ffo (KeyError)
        from test.rb:6:in `<main>'

@ksss
Copy link
Contributor Author

ksss commented Feb 11, 2016

I think need core help to support all methods that raise KeyError.
I proposed this https://bugs.ruby-lang.org/issues/12063

@kunitoo
Copy link

kunitoo commented Feb 11, 2016

Thank you for answer 😄
I understand the current situation.

@ksss
Copy link
Contributor Author

ksss commented Feb 16, 2016

I have solved the problem that remove self file backtrace at ksss@7314d4e .


error = assert_raises(KeyError) { hash.fetch("fooo") }
assert_correction %("foo"), error.corrections
assert_match %(Did you mean? "foo"), error.to_s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be interesting to say Did you mean? :foo (the symbol) when "foo" (the string) is passed.

@yuki24
Copy link
Member

yuki24 commented May 15, 2016

@ksss I'll merge this pull request once it's rebased.

@yuki24
Copy link
Member

yuki24 commented May 29, 2016

manually merged: 842b1e1, 3be5dca. Thank you for your contribution!

@yuki24 yuki24 closed this May 29, 2016
@ksss
Copy link
Contributor Author

ksss commented May 30, 2016

Thank you so much!

@ksss ksss deleted the key_error branch May 30, 2016 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants