Correctly handle settings.toml that ends without a newline #7381
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
\r\n files must be working due to micropython's built in handling of text-mode files, I didn't implement it. \r-only (old mac text-mode files) are explicitly not supported by the toml format.
dhalbert
approved these changes
Dec 26, 2022
|
@grbd if you can test with an artifact from this PR and let us know if the issue is fixed for you, that'd be helpful. Otherwise we'll probably merge it in the next day or so if we don't hear from you. |
|
I just tried testing by adding a couple of newlines and comments at the bottom of |
|
OK, I may be wrong about the cause then. I tested with a Pico W, perhaps there's a unique problem for esp32-s3 too. |
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.
The settings.toml parser did not handle a file that ended without a trailing newline. When such a file was encountered, retrieving the value on the last line would result in the error
Invalid byte 'EOF'. For values fetched automatically e.g., by web workflow this message could not be shown.However, such files are (A) frequently produced by text editors that folks actually use and (B) are specifically permitted by the TOML spec. (so my pedantic objection that (C) such files are not valid UNIX text files doesn't matter 馃槈)
I had hoped this would help with #7380 but the reporter tested and it did not.
This PR also adds printing of the failure messages parsing values fetched automatically. This helps with web workflow (e.g., if you have the incorrect line
CIRCUITPY_WIFI_SSID='example'you'll now seeAn error occurred while retrieving 'CIRCUITPY_WIFI_SSID': Invalid byte "'"each time the interpreter resets) however, values that are only fetched once like CIRCUITPY_RESERVED_PSRAM will probably still have their error messages hidden because they occur just once before the serial output is visible.DOS and UNIX-style line endings are both now explicitly tested, as is a no-eof file.
As a workaround, adding a comment line as the last line of the settings.toml should let it parse the last value.