Stream: IG creation
Topic: Comments in JSON
Chris Moesel (Apr 01 2020 at 16:46):
We would like some SUSHI-generated files to include a warning that the file is generated, so users should not directly edit it, else their changes may be overwritten. Usually this would be done using a comment, but JSON does not allow comments. The common Internet wisdom is that if you really need this (and can't use JSON5), use a special "//" or "_comment" key. In our case, "//" would make the most sense, since "_comment" could clash or be confused with a real field. For example, we would like to support this:
{
"//": "***********************************************************************************************",
"//": "* WARNING: DO NOT EDIT THIS FILE *",
"//": "* *",
"//": "* This file is generated by SUSHI. Any edits you make to this file will be overwritten. *",
"//": "***********************************************************************************************",
"resourceType": "ImplementationGuide",
"id": "hl7.fhir.us.sushi-test",
...
}
Chris Moesel (Apr 01 2020 at 16:48):
There are a few issues w/ this, however. The first if that the IG Publisher does not like duplicate keys -- even though the JSON spec does explicitly allow them (and most suggested comment workarounds do leverage this allowance):
Publishing Content Failed: Error parsing JSON source: Duplicated property name: // at Line 3
Chris Moesel (Apr 01 2020 at 16:50):
If we change it to use one "//" key with a value that is the array of comment strings instead, then the publisher completes, but we get this error in the qa.html: image.png
Lloyd McKenzie (Apr 01 2020 at 16:52):
Adding unexpected tags is explicity prohibited in FHIR. We could talk about adding something for FHIR R5, but we can't make it legal in prior versions. The only thing possible right now would be to send it as an extension.
Chris Moesel (Apr 01 2020 at 16:54):
I looked for a core extension that might be applicable but did not find one (and honestly, that's not the preferred approach anyway, as this shouldn't really be a formal component of the resource). Would it be legal to have the IG publisher automatically ignore (and toss) the "//" key? That way the resources that land in the final (IG Publisher produced) spec don't have these extra keys (so they're valid FHIR) -- but there's a way to have comments in dev resources...
Lloyd McKenzie (Apr 01 2020 at 16:58):
Not and be a legal R4 or earlier instance
Jean Duteau (Apr 01 2020 at 16:59):
If the IGPublisher took the input files and stripped "//" keys, then the resulting output files would be legal instances.
Chris Moesel (Apr 01 2020 at 17:01):
Jean Duteau said:
If the IGPublisher took the input files and stripped "//" keys, then the resulting output files would be legal instances.
Yeah, that's what I was thinking (and hoping for).
Jean Duteau (Apr 01 2020 at 17:01):
@Chris Moesel could you potentially get around this by generating XML files instead of JSON files? Then you could include comments. I know that XML just doesn't have the pizzazz these days, but...
Chris Moesel (Apr 01 2020 at 17:03):
@Jean Duteau -- yes, I suppose that's another approach. Since we use the TypeScript language (which is a superset of JavaScript), it's just so much easier for us to work in JSON. I guess we'd need to determine if the ROI is high enough. Currently, we're trying to come up w/ a solution we can implement quickly.
Jean Duteau (Apr 01 2020 at 17:05):
good point. I'm +1 on having the publisher strip comments :)
Grahame Grieve (Apr 01 2020 at 19:24):
some history on this:
- The lack of support for comments is absolutely the worst thing about json
- I'm not going to apologise for a validating parser prohibiting duplicate names
- we did define a special property name for comments in json, but the community insisted it be removed
- I can't see how JSON5 could be on the agenda
Jean Duteau (Apr 01 2020 at 19:25):
i think we (the sushi community) agree with all of that. we're just wondering if tooling could look for and remove a special tag when it is converting input instances to output instances?
Grahame Grieve (Apr 01 2020 at 19:29):
that's not very easy. The tooling is using the standard parsers, so you're asking for surgery down in the validator, since the validator loads all those resources
Grahame Grieve (Apr 01 2020 at 19:36):
not to mention needing a custom json parser
Grahame Grieve (Apr 01 2020 at 19:49):
(well, it we're going to allow comments we should do them for real)
Chris Moesel (Apr 01 2020 at 19:53):
OK. Fair enough. Thanks for considering. As for the parser, if you're using a 3rd party parser, I'm surprised it doesn't support duplicate keys -- as the JSON spec explicitly allows this and even notes that there are use cases in which duplicate keys are helpful. I assumed the duplicate key detection was added in FHIR code. In any case, it sounds like this is a no-go. We've found another workaround -- we'll put the warning in text.div but set text.status to empty so the IG Publisher overwrites our warning w/ the generated narrative.
Grahame Grieve (Apr 01 2020 at 19:58):
even notes that there are use cases in which duplicate keys are helpful
where?
Grahame Grieve (Apr 01 2020 at 20:08):
https://www.tbray.org/ongoing/When/201x/2013/02/21/JSON-Lesson
Grahame Grieve (Apr 01 2020 at 20:08):
but don't give up. I'm trying comments out
Chris Moesel (Apr 01 2020 at 20:29):
Uh... yeah. Well I feel dumb now. I think I misread a discussion on the spec -- and it really was why unique keys are helpful (not duplicate keys), so I got that totally backwards. I'm sorry for the mixup. Sometimes I get too excited and read things too fast I guess. But in the discussions on comment syntaxes, it seemed generally agreed upon that it was a good use case for duplicate keys. As noted in the blog you linked to, RFC8259 allows for it since it uses "should". In addition ECMA-404 ("The JSON Data Interchange Syntax") states:
The JSON syntax does not impose any restrictions on the strings used as names, does not require that name strings be unique, and does not assign any significance to the orderingof name/value pairs.
All that said, I'm fine w/ enforcing unique keys. I'm even more fine with it if I can use a single "//" key with a string or array of strings value. ;-)
Chris Moesel (Apr 01 2020 at 20:30):
@Grahame Grieve -- it seems that the IG publisher does allow me to get away with a single "//" property in package.json and package-list.json. If I do that will it cause any issues in other tools further down the line?
Grahame Grieve (Apr 01 2020 at 20:34):
it might. particularly package-list.json. Please don't make me test that. You shouldn't be generating package-list.json anyway
Chris Moesel (Apr 01 2020 at 20:38):
Right now, we basically generate a template of it because a lot of the values can be pulled from data in other places. We expect the author to ultimately take control of it, but we generate one when they don't provide it - as a kickstart. In the future, we'll be introducing a more comprehensive config file that can be used to reliably generate a package-list. So we want to make it clear when a specific package-list is output of SUSHI -- so authors edit the right thing.
Grahame Grieve (Apr 01 2020 at 20:39):
personally, I don't think you should try generating a complete one, since entries for part versions are always ignored during the publication process. As for generating the current... put a comment on there with instructions to remove the comment when you edit it - as you must
Chris Moesel (Apr 01 2020 at 20:40):
I think we're still figuring out how the integration all works and what the separation is between SUSHI input files, SUSHI output files, and IG Publisher input files. There are some gray lines right now and we're just trying to make things a little more clear while we go through some of these growing pains.
Grahame Grieve (Apr 01 2020 at 20:49):
well, as of next release, IG pbulisher will accept comments in json input resources
Grahame Grieve (Apr 01 2020 at 20:49):
genuine comments.
Grahame Grieve (Apr 01 2020 at 20:49):
per JSON5
Chris Moesel (Apr 01 2020 at 20:53):
Ha. From
I can't see how JSON5 could be on the agenda
to
as of next release, IG pbulisher will accept comments in json input resources
genuine comments.
per JSON5
in less than 90 minutes. Not bad! Thank you, @Grahame Grieve! Now I have to figure out how to support JSON5 comments in my stack...
Grahame Grieve (Apr 01 2020 at 20:55):
well, JSON5 isn't going to get on the general agenda, but I can implement a special mode for IG publisher. This only applies to R4+ btw
Chris Moesel (Apr 01 2020 at 20:55):
Got it. When you say "input resources", you mean stuff like package.json and package-list.json, but not necessarily resources (like ig-foo.json), right?
Grahame Grieve (Apr 01 2020 at 20:56):
not I mean resources in the FHIR sense
Chris Moesel (Apr 01 2020 at 21:08):
not I mean resources in the FHIR sense
Sorry, I'm having trouble interpreting that. Just to be clear, can you answer yes/no on these?
- Comment support in package.json?
- Comment support in package-list.json?
- Comment support in FHIR resource instances?
Grahame Grieve (Apr 01 2020 at 21:16):
package.json, package-list.json: comments not supported. And not going to be supported
resources : comments supported
Lloyd McKenzie (Apr 01 2020 at 21:27):
comments supported in source or comments supported in published instances?
Grahame Grieve (Apr 01 2020 at 21:47):
in the published resources
Lloyd McKenzie (Apr 01 2020 at 23:16):
Is that conformant with our published spec or an R5 change?
Grahame Grieve (Apr 01 2020 at 23:45):
of course not. So I wouldn't do it. That was just a April 1st answer. Comments are only supported in source
Jean Duteau (Apr 01 2020 at 23:45):
i was about to start the impeach process
Lloyd McKenzie (Apr 01 2020 at 23:51):
It's well past April 1 for you - cheater! :)
Last updated: Apr 12 2022 at 19:14 UTC
