Swapped decode/encode terms again #1025
Conversation
Synced jparse/ from the jparse repo (https://github.com/xexyl/jparse/). The term encode and decode were swapped back to the original meaning. This problem occurred due to the fact that when focusing for some time on the encoding/decoding bug of \uxxxx code points in the jparse repo the fact that it is a **JSON** encoder was lost in focus. This change also means that terms were swapped here. In the process of an issue in 'the other repo', some bugs were uncovered in jstrencode(1) but this should not, I believe, affect the timeline of the next IOCCC, fortunately. The website tools have to be updated which I will work on next a bit later on today.
|
I will do the website in a while ... afk now. |
|
Oh I forgot to run make release before I shut down. Okay I will fix this later. Sorry! |
|
QUESTION Should we have a new rule for the workflow that is like the slow one but in the case where it fails it cats the build log? I think that's a good idea. I will consider doing that later after I fix this issue whilst waiting for make www to do its thing in the other repo. |
Rushing to get the fix described in commit c64884e803934da0240fe994644d26040037f85a in the jparse repo I forgot I had changes I had been playing with (which helped to uncover the newly discovered bugs but which is nonetheless incorrect) and these changes broke jstr_test.sh. This should fix the problem.
|
That stupid problem was fixed. Due to trying to get a commit in quickly I forgot I had other changes in the file ... which is unfortunate. But they did help uncover some bugs in the encode tool. Doing UPDATE 0... and of course there was an error. Just noticed that. Fixed and trying again. |
Sadly yes. That's what can happen when you're so focused on one thing that you lose sight of something else. Have to check on make www again ... I would not be surprised if there are other problems to work out. |
|
.. so far so good though. |
|
However ... $ jstrencode hi
hiTo be properly JSON encoded, it should produce: |
|
And: $ jstrdecode hi
hishould have produced an error, as hi is NOT a valid JSON encoded string. And: $ jstrdecode '"hi"'
Warning: json_decode: found \-escaped char: "
Warning: main: error while decoding processing arg: 0
Is not correct because: is a proper JSON encoded string. |
That's true. I wonder when that cropped in. |
|
Actually I seem to recall the second one always did work. Not sure. |
|
Yes the |
As far as this goes. I recall this being in here for months, perhaps always. Maybe I can look into it but not now. |
|
Okay the problem is it's supposed to be escaped. It's an easy fix but is it right or not to remove that check? I don't know. Please advise. |
|
This diff fixes it btw: diff --git i/json_parse.c w/json_parse.c
index 0aa28e5878bf9361a94dcbecafb01df0f9b8a988..a6e183c7567c45f5316849cd25cfe4afece5e539 100644
--- i/json_parse.c
+++ w/json_parse.c
@@ -1700,11 +1700,10 @@ json_decode(char const *ptr, size_t len, size_t *retlen)
*retlen = 0;
}
warn(__func__, "found non-\\-escaped char: 0x%02x", (uint8_t)c);
return NULL;
break;
- case '"': /*fallthrough*/
case '\\':
/* error - clear allocated length */
if (retlen != NULL) {
*retlen = 0;
}
|
|
I think it could be a bigger problem. In some cases it should not be escaped like the example you gave. But in other times it should be like inside a string. Maybe the decode tool needs an option to pass to the function to decide whether to ignore it or not? Though that could get kind of ugly. Let me know how you wish to proceed .. or when back open a pull request yourself. |
Well I don't see why since everything is going as planned with the updates I have made. Reverting would also lose other changes too, quite a few important ones. |
|
We will be happy to look at the state of things in about 2 days. Best wishes. |
You too! |
With the commit I made a short bit ago you can do: jstrencode -s hiand get: $ jstrencode -s hi
"hi"As noted elsewhere, the reason for not assuming a string by default is there are many other options and it would be annoying to have to specify an option to disable it in the case that it's not a string; there are, after all, many types, not just one. |
As for this ... I think that I have to update the
Try: $ jstrdecode -s '"hi"'
hiThe reason for |
Oops .. this is also fixed actually. Same option as both are to do with whether or not it's a string. All should be good now unless some other issue is found, plus of course the issues I discovered yesterday with jstrdecode .. or encode, don't remember: was a long day and early morning. |
|
On the other hand ... this still seems like an issue that needs to be addressed, depending on desire. I'll make a comment over there. |
Synced jparse/ from the jparse repo (https://github.com/xexyl/jparse/). The term encode and decode were swapped back to the original meaning. This problem occurred due to the fact that when focusing for some time on the encoding/decoding bug of \uxxxx code points in the jparse repo the fact that it is a JSON encoder was lost in focus. This change also means that terms were swapped here.
In the process of an issue in 'the other repo', some bugs were uncovered in jstrencode(1) but this should not, I believe, affect the timeline of the next IOCCC, fortunately. The website tools have to be updated which I will work on next a bit later on today.