Handle non-printable control characters when escaping JSON#7435
Merged
zth merged 5 commits intorescript-lang:masterfrom May 7, 2025
Merged
Handle non-printable control characters when escaping JSON#7435zth merged 5 commits intorescript-lang:masterfrom
zth merged 5 commits intorescript-lang:masterfrom
Conversation
mediremi
commented
May 6, 2025
mediremi
commented
May 6, 2025
| @@ -1,153 +1,14 @@ | |||
| let ( >:: ), ( >::: ) = OUnit.(( >:: ), ( >::: )) | |||
| type t = Ext_json_noloc.t | |||
Member
Author
There was a problem hiding this comment.
The existing Ext_json tests were moved to tests/ounit_tests/ounit_ext_json_tests.ml
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
zth
approved these changes
May 7, 2025
Member
zth
left a comment
There was a problem hiding this comment.
This is looking good to me, thank you! @cristianoc any thoughts?
Collaborator
Looks great! |
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.
When the VS Code extension calls
rescript-editor-analysis, it replies with JSON serialised by analysis/vendor/json/Json.ml.While
Json.escapeescapes printable characters correctly, it did not handle non-printable control characters (i.e. characters with a char code < 0x20). This meant that JSON sent to VS code was not always escaped properly and prevented code actions from being available in certain situations.For example, opening up https://github.com/rescript-lang/rescript-lang.org/blob/master/src/bindings/RescriptCompilerApi.res in VS Code currently results in
SyntaxError: Bad control character in string literal in JSON at position 1319 (line 15 column 751):In the example above, this is caused by
Warning.toCompactErrorLinecontaining a string with control character0x27.With the escaping logic I've added, this issue no longer occurs.
Changes I've made
analysis/vendor/json/Json.mltests/ounit_tests/ounit_json_tests.mltotests/ounit_tests/ounit_ext_json_tests.mltests/ounit_tests/ounit_json_tests.mltest suite foranalysis/vendor/json/Json.mlcompiler/ext/ext_json_noloc.ml, which is used by the bsb watcherFuture work
analysis/vendor/json/Json.mlandcompiler/ext/ext_jsonshould probably be eventually replaced with Yojson