Repository navigation
Save history independently of the locale encoding - #1277
Open
ihamzatarar wants to merge 1 commit into
Open
ihamzatarar wants to merge 1 commit into
ihamzatarar wants to merge 1 commit into
Conversation
Opening the history file with the locale's encoding makes f.puts raise Encoding::UndefinedConversionError for any entry holding a character the locale cannot represent. The rescues around it only cover Errno, so the exception escapes save_history, which runs from the ensure block in IRB.run, and IRB aborts on exit with the whole session discarded. Write the entries in the encoding they already carry instead. Fixes ruby#1023
This branch has not been deployed
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
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.
Fixes #1023.
Under a locale that cannot represent what was typed, IRB aborts on exit and the session's history is thrown away:
save_historyopens the file withencoding: IRB.conf[:LC_MESSAGES]&.encoding, which resolves to US-ASCII here, sof.puts(hist)has to transcode and raises on the first character the locale has no room for. The two rescues below it coverErrno::ENOENT,Errno::EACCESandErrno::EPERM, so this one escapes.save_historyis called from theensureblock inIRB.run, which is why the whole session goes rather than just the write.l.scrubon the line above does not help: it repairs invalid bytes, and these bytes are perfectly valid UTF-8. There is simply no US-ASCII character to transcode them to.This writes the entries in the encoding they already carry, so nothing has to be converted and nothing is lost. The history file is IRB's own file, so there is no external consumer whose expectations the byte encoding has to match.
Scope, since it is worth being explicit about what this does not touch.
load_historyhas the mirror problem one line up, atFile.open(history_file, "r:#{IRB.conf[:LC_MESSAGES].encoding}"): an existing UTF-8 history file read back under an ASCII locale comes through with its characters mangled. That path is covered today bytest_history_different_encodings, which asserts the mangling, so changing it would mean deciding that the current behaviour is wrong as well. Reading is lossy but does not raise, and the crash and the data loss on exit come entirely from the write. I have kept this change to that and am happy to follow up on the read side separately if you would like it changed.The new test puts a non-ASCII entry in the history with
LC_MESSAGESset toen_US.ASCIIand asserts the save does not raise and the entry survives. On master it fails withEncoding::UndefinedConversionError: U+00E9 from UTF-8 to US-ASCII.The full suite goes from 402 tests and 2483 assertions to 403 and 2484, with the same single pre-existing failure either way (
test_use_tracer_enabled_when_gem_is_available, which needs the tracer gem).