Fix methods defined with invalid encoding are not displayed in completion#1101
Merged
Conversation
ksaito422
force-pushed
the
invalid-encoding-method
branch
3 times, most recently
from
8fc5770 to
26a1d04
Compare
ksaito422
force-pushed
the
invalid-encoding-method
branch
from
26a1d04 to
04d2cf5
Compare
tompng
reviewed
Jun 14, 2025
| i.encode(Encoding.default_external) | ||
| rescue Encoding::UndefinedConversionError | ||
| warn "Warning: Invalid encoding in method name '#{i}'. can't be converted to the locale #{Encoding.default_external}." unless @encoding_warning_shown | ||
| @encoding_warning_shown = true |
Member
There was a problem hiding this comment.
It's not possible to show a warning safely here. I think we can just silently ignore invalid ones.
Contributor
Author
There was a problem hiding this comment.
Fixed to ignore invalid encoding.
f8ed14a
tompng
reviewed
Jun 14, 2025
Comment on lines
+199
to
+200
| i.encode(Encoding.default_external) | ||
| rescue Encoding::UndefinedConversionError |
Member
There was a problem hiding this comment.
Can you add this encode and rescue to IRB::TypeCompletor#completion_candidates?
It is the default completor when RUBY_VERSION >= '3.4'
Contributor
Author
There was a problem hiding this comment.
Applied the same fix to IRB::TypeCompletor#completion_candidates.
4897ccf
Contributor
Author
|
The CI failures don't seem related. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a crash in IRB's method completion when a completion candidate contains characters that cannot be converted to
Encoding.default_external.（#900）Problem
Currently, if a method or variable name includes characters incompatible with
Encoding.default_external, triggering method completion results in anEncoding::UndefinedConversionError, which crashes the entire IRB session. This leads to a poor user experience, as the session terminates unexpectedly and work can be lost.Solution
Instead of allowing the exception to crash the session, this change catches the encoding error and prints a warning message to stderr. To avoid flooding the console, this warning is displayed only once per session upon the first occurrence of the error.
This approach is preferable because it informs the user about the underlying encoding issue without abruptly ending their workflow.
Behavior Change