Fix unicode issues with shortcuts.run #127
Closed
Conversation
This file contains 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
When there is an incomplete multibyte sequence, process the data only until the start of this sequence. When next chunk is read, prepend the left overs to it. This should complete the sequence and processing should continue normally. Fixes: release-engineering#119
universal_newlines is only one of several arguments which activate text mode with Popen; make sure we support all of them (otherwise crashes occur when those arguments are passed). Rather than catching decoding errors and keeping track of remaining data explicitly, let's use a stream reader class designed for that purpose. Let's document the behavior that the process must always output UTF-8. Fixes release-engineering#126
|
I was playing around with an alternative solution for #120, but looks like it needs more attention on the log file handling. |
|
With this patch the output of the command would always be decoded as UTF-8. That might break some use-cases. With current code it should be possible to run commands that return even binary data as long as Your approach has the advantage that the log file could be opened in text mode unconditionally and you would always send unicode to it. Currently it's trying to match the |
|
Not planning to proceed with this. |
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.
Merge request includes fixes for: