-
Notifications
You must be signed in to change notification settings - Fork 137
fix: don't crash on utf18 autocompletion #1129
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -280,7 +280,7 @@ def retrieve_completion_data(input, bind:, doc_namespace:) | |
| rescue EncodingError | ||
| # ignore | ||
| end | ||
| candidates.grep(/^#{Regexp.quote(sym)}/) | ||
| safe_grep(candidates, /^#{Regexp.quote(sym)}/) | ||
| end | ||
| when /^::([A-Z][^:\.\(\)]*)$/ | ||
| # Absolute Constant or class methods | ||
|
|
@@ -291,7 +291,7 @@ def retrieve_completion_data(input, bind:, doc_namespace:) | |
| if doc_namespace | ||
| candidates.find { |i| i == receiver } | ||
| else | ||
| candidates.grep(/^#{Regexp.quote(receiver)}/).collect{|e| "::" + e} | ||
| safe_grep(candidates, /^#{Regexp.quote(receiver)}/).collect{|e| "::" + e} | ||
| end | ||
|
|
||
| when /^([A-Z].*)::([^:.]*)$/ | ||
|
|
@@ -378,7 +378,7 @@ def retrieve_completion_data(input, bind:, doc_namespace:) | |
| if doc_namespace | ||
| all_gvars.find{ |i| i == gvar } | ||
| else | ||
| all_gvars.grep(Regexp.new(Regexp.quote(gvar))) | ||
| safe_grep(all_gvars, Regexp.new(Regexp.quote(gvar))) | ||
| end | ||
|
|
||
| when /^([^.:"].*)(\.|::)([^.]*)$/ | ||
|
|
@@ -453,16 +453,30 @@ def retrieve_completion_data(input, bind:, doc_namespace:) | |
| else | ||
| candidates = (bind.eval_methods | bind.eval_private_methods | bind.local_variables | bind.eval_instance_variables | bind.eval_class_constants).collect{|m| m.to_s} | ||
| candidates |= RubyLex::RESERVED_WORDS.map(&:to_s) | ||
| candidates.grep(/^#{Regexp.quote(input)}/).sort | ||
| safe_grep(candidates, /^#{Regexp.quote(input)}/).sort | ||
| end | ||
| end | ||
| end | ||
|
|
||
| # Set of available operators in Ruby | ||
| Operators = %w[% & * ** + - / < << <= <=> == === =~ > >= >> [] []= ^ ! != !~] | ||
|
|
||
| def safe_grep(candidates, pattern) | ||
| target_encoding = Encoding.default_external | ||
| candidates.filter_map do |candidate| | ||
| next unless candidate | ||
|
|
||
| converted = candidate.encoding == target_encoding ? candidate : candidate.encode(target_encoding) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Encoding conversion is more than safe-grep. I think this method should only do safe grep ( Encoding conversion is already done in line 205. def completion_candidates(preposing, target, postposing, bind:)
...
completion_data = retrieve_completion_data(target, bind: bind, doc_namespace: false).compact.filter_map do |i|
i.encode(Encoding.default_external)
rescue Encoding::UndefinedConversionError
# If the string cannot be converted, we just ignore it
nil
end
...
end |
||
|
|
||
| converted if pattern.match?(converted) | ||
| rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError, Encoding::CompatibilityError, EncodingError | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| # Skip candidates that cannot be converted to the target encoding | ||
| nil | ||
| end | ||
| end | ||
|
|
||
| def select_message(receiver, message, candidates, sep = ".") | ||
| candidates.grep(/^#{Regexp.quote(message)}/).collect do |e| | ||
| safe_grep(candidates, /^#{Regexp.quote(message)}/).collect do |e| | ||
| case e | ||
| when /^[a-zA-Z_]/ | ||
| receiver + sep + e | ||
|
|
||
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.
This part is not needed.
Array#grepaccepts nil, but all candidates passed to this method is always an array of string.Uh oh!
There was an error while loading. Please reload this page.
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 was surprised as well but without this the tests fail on TruffleRuby (compare the builds on the two commits in this PR).
It's working on every other platform though. 🤔
Or was I missing a bit here or misunderstanding something?
/CC @eregon
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.
Sorry, my comment "always an array of string" was wrong.
Changing this
collecttofilter_mapto reject nil seems better.