Leading 0 in property makes generation fail
Closed this issue · 4 comments
If a jsonl property has a leading 0 (for text alignment purposes) the generation will fail with
json.decoder.JSONDecodeError: Expecting ',' delimiter: line 1 column 58 (char 57)
e.g: {"page":2,"id":25,"obj":"label","x":02,"y":030,"w":033,"h":029,"border_width":01}
Strictly speaking JSON syntax doesn't allow numbers to start with the digit 0.
You can use a space for alignment or put your numbers in quotes.
Hmm yeah I am not entirely sure about this. openhasp-config-manager already allows things within the jsonl files that would normally be invalid, so I think its not unreasonable to allow leading zeros in the "source" and just strip them out during normalization. I also really don't like how a comma on the last line will break the parsing, even though its just style and doesn't affect parsing in any way. (I know, this is in the JSON spec as well)
Like @fvanroie mentioned you can use a string instead, openhasp-config-manager automatically will try to convert the value into the type that is expected by openHASP for a given parameter. F.ex. x, y, h, w will all be converted to an integer during normalization. The reason this was added initially, was because I didn't want to make a jinja template out of the whole jsonl file, since that would completely break syntax highlighting (at least in pycharm). For that reason you have to specify any template as a string, so the value has to be converted to the correct type after template rendering anyway.
You are correct about it being invalid json and not really a big deal.
I just wanted to mention what broke when I moved my openhasp code to your manager.
Trailing commas on the last object property should now be stripped away before parsing.
I have thought about creating a preprocessor to remove leading zeros from numbers, but I have decided against it, because I feel like it would be pretty invasive and could potentially lead to a lot of bugs, especially when considering the fact that the configuration files do not only contain json but also jinja templates. So I will not add something like this for now.
If this comes up again we can reevaluate how useful and feasible it would be. Until then I will close this issue.
Thank you for reporting this! ❤️