YAML now loads all strings as unicode #592
Conversation
- All strings are redirected to be loaded as unicode - I was experiencing an issue on my system where I was getting errors trying to access config file objects because they were not being closed. In all cases, once the YAML content was loaded, we were done accessing the file content. YAML object will close file access in the objects once processing the content is complete.
|
Looks like everything checked out in CI. I probably won't be able to address any issues until late tonight my time, but I think this is a pretty straight forward pull. |
|
As an interesting test of this, I would like to see if the tests still pass with the changes to Also, this PR should probably have a note added to the release notes. One other potential concern: what happens when a user passes a value in on the command line? Specifically, is it Unicode? I realize only a few of the config settings are even available from the command line, but it might be worth looking into. I have not checked this myself. |
Yes removing
I can do that later today.
I have not checked this myself either. Though that is a general Unicode support thing. This issue is specifically just enforcing an all Unicode yaml load, not to bring mkdocs up to a 100% Unicode safe state. I think eventually an in-depth analysis of Unicode handling should be performed, but this is probably not the place for that. What happens after the load needs to be looked at separately, but it is definitely something important to look into, especially as we are now enforcing |
|
|
||
| class Loader(loader): | ||
|
|
||
| """Custom Loader.""" |
There was a problem hiding this comment.
This comment can be removed, it doesn't tell me anything.
There was a problem hiding this comment.
Sure, I'm just in a habit now of providing comments for classes and methods, I kill it in the next rev.
|
Other than the comment update and a quick addition to the release notes, this looks great. Thanks very much! I'll merge it right after you update. I'd like to remove the finally that closes files, but I'll do that in a follow up. It should be the responsibility of the tests to clean up. Thanks! |
Yeah, my intentions wasn't to have it there at all, but I realized I would have to massage some other things outside the scope of this pull to have it not there, and I was trying to get this in today as I knew you were prepping for a bug fix, major release? I will wrap this up tonight. |
-Reorganize the yaml loading function to make it more clear what is happening. -Comment more to to make explicitly clear what is happening. -Remove unnecessary 'pass' -Make a note that 'finally' should be removed when the root of the issue is cleared up. -Update release notes with info on change.
YAML now loads all strings as unicode
|
Great! Thanks! |
trying to access config file objects because they were not being closed.
In all cases, once the YAML content was loaded, we were done accessing
the file content. YAML object will close file access in the objects
once processing the content is complete.