ioccc-src / mkiocccentry

Form an IOCCC entry as a compressed tarball file
Other
28 stars 5 forks source link

Add struct json *node to some struct json_ structs #833

Closed xexyl closed 9 months ago

xexyl commented 9 months ago

The following struct json_foo structs were modified to have a struct json *node so one can make use of the parse functions of a type of struct json_foo for explicit parsing of a JSON node outside of the parsing of a document:

json_number
json_string
json_boolean
json_null

The idea of parsing a specific JSON node outside of a document is for the jval and jnamval options -S and -n which have to verify that the values specified in the options are valid JSON as the comparison will need the values in the structs json_string and json_number depending on the options of the tools. This feature might be useful in other cases as well but at this point only jval and jnamval use it.

The function json_alloc() explicitly sets node to NULL and the parsejson(number|string|boolean|null) set the item->node to the respective struct json *node.

It might be that other json_ structs need to be modified but this is TBD later. If so the functions json_alloc() and the respective parsejson will have to be updated in a similar manner as the above mentioned ones.

The tool jnum_gen has been updated to explicitly set the node to NULL in the test_result array. Updated jnum_test.c through make rule rebuild_jnum_test.

This member to the structs has not been added to the man page. Whether this should be done depends on how the man page (jparse.3) is constructed but if it does not explicitly list the struct json_ structures it might be good to add them. This would also be good to add to the README.md file.

Fix make picky for jnamval_util.h which happened because of the prefix name change of JNAMVALPRINT to JNAMVALRESTRICT. This was done by changing two (though only one line was a problem) macros which are just some bitwise ORs to be two lines instead of one (the usual \ at the EOL).

New version of JSON parser and jparse: 1.1.5 2023-08-08.

New version of jval and jnamval: 0.0.14 2023-08-08.

New version of jnum_gen: 1.0.1 2023-08-08.

xexyl commented 9 months ago

Did I miss a struct that needs this? Can you think of a better way to go about it? I'm going to rest now but I'll fix if you have another idea.

lcn2 commented 9 months ago

See comment 1669327108.

We are inclined to decline this pull request. Not only is the impact of on the JSON tree significant and needs to be carefully considered, but the this moves into implementation of utilities that are still being designed, is well beyond what is needed to parse the command line, and is venturing into how JSON trees are searched and matched.

xexyl commented 9 months ago

Converting to draft as you seem to have a better idea ... though I think that parsing individual json nodes is a great way to verify that the values of -S and -n are valid JSON values. It also sets up the right booleans and variables to use. Later on it should prove useful to print out too using the print functions.

Let me know if you wish me to close this but if so it will greatly complicate parsing of the options -S and -n to verify that they're valid.

xexyl commented 9 months ago

See comment 1669327108.

We are inclined to decline this pull request. Not only is the impact of on the JSON tree significant and needs to be carefully considered, but the this moves into implementation of utilities that are still being designed, is well beyond what is needed to parse the command line, and is venturing into how JSON trees are searched and matched.

Doing this will greatly complicate parsing of the options. See my comments in the other thread. I'll close this and we can discuss it later. But wasn't the option parsing supposed to parse without implementation? That does it.

lcn2 commented 9 months ago

We are closing this pull request until it can be further discussed, sorry.

xexyl commented 9 months ago

Okay well in that case the parsing of -S and -n are no longer complete and will take more time to consider. Going to rest now ... will have to think of how to do it or else discuss it with you. That likely won't happen for a while, the parsing of the options. I think that's unfortunate but maybe discussion is better.

Time to rest.

lcn2 commented 9 months ago

Sorry. Going to rest as well.

xexyl commented 9 months ago

Sorry. Going to rest as well.

I'm happy to hear that.

xexyl commented 9 months ago

I've left what I hope are some useful updates in the issue thread. I'll look back tomorrow and hopefully can look and reply in more detail. Hope you have a good day.

I might manage to reply to the colour thing but I don't even know about that. I actually forgot about it until just now. I had some thoughts but I think I'll have to return to that another time too.