Hi Theresa, thank you for your review, which highlights a number of needed improvements. > Minor issues: > > Section 1: > Please consider adding a brief description of what a control operator is, how > it can be used, and what its components are. I’m not so sure about that. This document really is not useful without some knowledge of RFC 8610, and the use of control operators is detailed there. Paraphrasing a normative document in another normative document always has this danger of subtly changing things… I added a reference to Section 3.8 of RFC8610 to the first sentence. Now https://github.com/cbor-wg/cddl-control/commit/036f6d8 > .feature is described as "Detecting feature use in extension points" > (introduction) VS "indicating the use of a non-basic feature in an instance" > (abstract) - How can a control operator detect feature use, doesn't it merely > indicate? If both of these descriptions are meant to conved the same meaning, > please consider changing the "purpose" of .feature to something like "Name of a > feature that is used", or, ideally, find a different word to capture what a > feature is. Good point. I don’t think losing the term “extension point”, so I changed this into: | .feature | Indicate name of feature used (extension point) | It is always hard to cram all the intended meaning into a table cell, but I think this should be closer. Now https://github.com/cbor-wg/cddl-control/commit/909b407 > Section 2: > "CDDL as defined in [RFC8610] does not have any mechanisms to compute > literals. As an 80 % solution, this specification adds three control > operators" > What are the missing 20%? Please consider briefly mentioning the limitations of > the new operators. Indeed, that wording was noticed by others, too. https://github.com/cbor-wg/cddl-control/commit/87ad909 changes this into: -literals. As an 80 % solution, this specification adds three control +literals. To cover a large part of the use cases, this specification adds three control I think it should be obvious that numeric addition and string concatenation (possibly dedented) does not cover everything that a full-blown expression language could (which would need to wait for CDDL 2.0 if we were going to create that). > "Not all tools may be able to work with non-unique targets or controllers." > Here, "tool" refers to "CDDL tool" as used in RFC8610, correct? If so, please > consider making all references to "tool" in this document more precise. I put this into the terminology (after the reference to RFC 8610 terminology): +"Tool" refers to tools along the lines of that described in {{Appendix F of -cddl}}. Now https://github.com/cbor-wg/cddl-control/commit/6241afe > Section 2.3: > Please consider adding the outcome of the .det operation in the example in > Figure 3: The definition of dedenting includes "determining the smallest amount > of left-most blank space (number of leading space characters) in all the > non-blank lines". If I understand correctly, taking this definition literally, > the operation ("oid" .det cbor-tags-oid) would result in no space being removed > at all, because the string "oid" does not contain anly left-most blank spaces. The text says that "both arguments (target and controller) are independently dedented before the concatenation takes place”, so that rules out this interpretation. But examples always help, so I added: For the first rule in {{exa-det}}, the result is equivalent to {{exa-det-result}}. ~~~~ cddl oid = bytes .abnfb 'oid oid = 1*arc roid = *arc arc = [nlsb] %x00-7f nlsb = %x81-ff *%x80-ff ' ~~~~ {: #exa-det-result title="Dedenting example: result of first .det”} Now https://github.com/cbor-wg/cddl-control/commit/de9bc15 > But I suspect that the example is intended to show dedenting of the contents of > the cbor-tags-oid variable. Perhaps I'm missing something about CDDL here that > was discussed in RFC8610. > > Section 7 (Security considerations): > Can there be any additional security concerns if CDDL specifications can > contain ABNF or "arbitrary" features? While this document obviously can't go > into specifics, maybe it's worth calling out that one needs to pay specific > attention if these control operators are used. Isn’t that true for any extension to any language? I’d prefer to highlight specific security considerations when I’m aware of them. (I wouldn’t go as far as RFC 5234, though: 5. Security Considerations Security is truly believed to be irrelevant to this document. :-) > Nits/editorial comments: > > Section 2.2 > "concatenating the target text string ""foo""" > Is foo intended to be in two double quotes, or should there only be one pair of > quotes? This is an artifact of the way xml2rfc handles <tt> elements. There currently is no way to mark the document up in a way such that both plain text and html versions look good, and I opted to prefer the HTML to look good (and correct). (RFC 8949 has a long explanation in several paragraphs of Section 1.2 how the plain text sometimes will look weird; I was hoping I wouldn’t have to repeat this here.) There is a proposed change to xml2rfc that will change the specifics, so I would like to wait for how that plays out before making changes. > Section 3 > "by defining a ".abnf" control operator" > Should this say 'an ".abnf" control operator' instead? Actually, we say out loud “dot a b n f control operator”, so “a” corresponds to the speech pattern. The changes are collected in the “TE-…” commits in https://github.com/cbor-wg/cddl-control/pull/11 Grüße, Carsten -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
