conjure
conjure copied to clipboard
Escape more characters in JSON output. Fixes #517
#517
Disclaimer: This was implemented entirely using GitHub Codespaces (without sufficient RAM to build the whole project) on my mobile phone during toilet breaks with zero experience in Haskell. Please be gentle!
Ah, small mistake in the commit message, I meant to note that it only addresses the first point about the backslashes.
Also, I haven't checked if there's anywhere else where escaping is performed, which could lead to double escaping. Trusting the unit tests will catch that. (Maybe I should add my own unit test even)
Can you see the CI output @stylpe ? After clicking a few times, I get to this page: https://dev.azure.com/conjure-testing/conjure/_build/results?buildId=442&view=logs&jobId=b227660a-cc34-513e-56b6-a239a911c194&j=b227660a-cc34-513e-56b6-a239a911c194&t=778185b6-24a8-5c4e-9ea7-ecd373f7924f
Which gives a compilation error:
/home/vsts/work/1/s/src/Conjure/Language/Pretty.hs:124:13: error:
• Variable not in scope: singleton :: Char -> Text
• Perhaps you meant one of these:
‘T.singleton’ (imported from Data.Text),
‘V.singleton’ (imported from Data.Vector),
‘M.singleton’ (imported from Data.HashMap.Strict)
|
124 | jsonEsc c = singleton c
| ^^^^^^^^^
Yep, already made one amendment based on the CI logs :)
The error message is particularly helpful this time, it correctly suggests T.singleton
as a fix! Will commit and push in a moment.
And look, the language service finished installing overnight so I can see it in the editor now!
Ha, speaking of insufficient RAM, you might need to size up the builders:
ghc.exe: getMBlocks: VirtualAlloc MEM_COMMIT failed: The paging file is too small for this operation to complete.
The windows builder on Azure has been unreliable, yes. The linux/max ones should work fine I am hoping.
Here's a minimal failing repro model, but I'm not 100% sure where to add it, is tests/pretty_print/issues/ ok?
$ The variant triggers refinement output that is vulnerable to the bug
find x : variant {i:int(1..3),b:bool}
$ The backslash at the end of the next line doesn't get eacaped, producing "... /\"
such that active(x,i) -> x[i]>2 /\
x[i]<5
$ Simpler cases that fail since `\ ` isn't a valid JSON escape sequence
find y:bool such that y /\ true
find z:bool such that and([z,true])
The parse_print directory is quite specialised for testing the roundtrip: parse/print/parse is the same as parse.
You can put it under tests/custom/issues/519
if you like? You'll have to put a run.sh file in there as well.
I learned how to use git rebase -i
, so I fixed up the branch a bit. The added test is a blind commit again, so relying on CI for feedback, otherwise I consider this ready!
This time it should be OK!
Thanks for the contribution @stylpe!