Support lists and code blocks in doc.go
Our documentation comments already include examples of code blocks
and lists, they just don't get rendered right. We also have things that
were trying to be lists but aren't. Go ahead and add support for it, and
fix the handful of list-like things that didn't get rendered as lists.
I took inspiration from CommonMark (https://spec.commonmark.org/0.30/)
to resolve questions such as whether blank lines are needed between
lists, etc., but this does not support any kind of nesting and is still
far from a CommonMark parser. Aligning with CommonMark leaves the door
open to pulling in a real Markdown parser if we start to need too many
features. I've also borrowed the "block" terminology from CommonMark.
One ambiguity of note: whether lists may interrupt paragraphs (i.e.
without a blank line in between) is a little thorny. If we say no, this
doesn't work:
Callers should heed the following warnings:
1) Don't use the function
2) Seriously, don't use this function
3) This function is a bad idea
But if we say yes, this renders wrong:
This function parses an X.509 certificate (see RFC
5280) into an X509 object.
We have examples of both in existing comments, though we could easily
add a blank line in the former or rewrap the latter. CommonMark has a
discussion on this in https://spec.commonmark.org/0.30/#lists
CommonMark says yes, but with a hack that only lists starting with 1 can
interrupt paragraphs. Since we're unlikely to cite RFC 1, I've matched
for now, but we may want to revisit this if it gets to be a pain. I
could imagine this becoming a problem:
This function, on success, does some stuff and returns
1. Otherwise, it returns 0.
But that looks a little weird and we usually spell out "one" and "zero".
I printed all the lists we detected in existing comments, and this has
not happened so far.
I've also required fewer spaces than CommonMark to trigger a code block.
CommonMark uses four, but four spaces plus a leading "//" and a " " is
quite a lot. For now I'm not stripping the spaces after the comment
marker at comment extraction time and then requiring three spaces, so
two spaces relative to normal text. This is mostly to match what we've
currently been doing, but we can always change it and our comments
later.
Change-Id: Ic61a8e93491ed96aba755aec2a5f32914bdc42ae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64930
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/bn.h b/include/openssl/bn.h
index 92ff7a1..fb6e962 100644
--- a/include/openssl/bn.h
+++ b/include/openssl/bn.h
@@ -666,11 +666,11 @@
// The callback receives the address of that |BN_GENCB| structure as its last
// argument and the user is free to put an arbitrary pointer in |arg|. The other
// arguments are set as follows:
-// event=BN_GENCB_GENERATED, n=i: after generating the i'th possible prime
+// - event=BN_GENCB_GENERATED, n=i: after generating the i'th possible prime
// number.
-// event=BN_GENCB_PRIME_TEST, n=-1: when finished trial division primality
+// - event=BN_GENCB_PRIME_TEST, n=-1: when finished trial division primality
// checks.
-// event=BN_GENCB_PRIME_TEST, n=i: when the i'th primality test has finished.
+// - event=BN_GENCB_PRIME_TEST, n=i: when the i'th primality test has finished.
//
// The callback can return zero to abort the generation progress or one to
// allow it to continue.
diff --git a/include/openssl/curve25519.h b/include/openssl/curve25519.h
index a455389..b87df52 100644
--- a/include/openssl/curve25519.h
+++ b/include/openssl/curve25519.h
@@ -161,10 +161,10 @@
// |*out_key_len| to the number of bytes written.
//
// The resulting keying material is suitable for:
-// a) Using directly in a key-confirmation step: i.e. each side could
+// - Using directly in a key-confirmation step: i.e. each side could
// transmit a hash of their role, a channel-binding value and the key
// material to prove to the other side that they know the shared key.
-// b) Using as input keying material to HKDF to generate a variety of subkeys
+// - Using as input keying material to HKDF to generate a variety of subkeys
// for encryption etc.
//
// If |max_out_key_key| is smaller than the amount of key material generated
diff --git a/include/openssl/ec.h b/include/openssl/ec.h
index 2d005af..e8a2db0 100644
--- a/include/openssl/ec.h
+++ b/include/openssl/ec.h
@@ -121,10 +121,10 @@
// calling |EC_GROUP_free| is optional.
//
// The supported NIDs are:
-// NID_secp224r1 (P-224),
-// NID_X9_62_prime256v1 (P-256),
-// NID_secp384r1 (P-384),
-// NID_secp521r1 (P-521)
+// - |NID_secp224r1| (P-224)
+// - |NID_X9_62_prime256v1| (P-256)
+// - |NID_secp384r1| (P-384)
+// - |NID_secp521r1| (P-521)
//
// Calling this function causes all four curves to be linked into the binary.
// Prefer calling |EC_group_*| to allow the static linker to drop unused curves.
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index ab6225e..ff4d4c9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1461,19 +1461,19 @@
//
// Available opcodes are:
//
-// The empty opcode enables and appends all matching disabled ciphers to the
+// - The empty opcode enables and appends all matching disabled ciphers to the
// end of the enabled list. The newly appended ciphers are ordered relative to
// each other matching their order in the disabled list.
//
-// |-| disables all matching enabled ciphers and prepends them to the disabled
+// - |-| disables all matching enabled ciphers and prepends them to the disabled
// list, with relative order from the enabled list preserved. This means the
// most recently disabled ciphers get highest preference relative to other
// disabled ciphers if re-enabled.
//
-// |+| moves all matching enabled ciphers to the end of the enabled list, with
+// - |+| moves all matching enabled ciphers to the end of the enabled list, with
// relative order preserved.
//
-// |!| deletes all matching ciphers, enabled or not, from either list. Deleted
+// - |!| deletes all matching ciphers, enabled or not, from either list. Deleted
// ciphers will not matched by future operations.
//
// A selector may be a specific cipher (using either the standard or OpenSSL
@@ -1483,36 +1483,36 @@
//
// Available cipher rules are:
//
-// |ALL| matches all ciphers, except for deprecated ciphers which must be
+// - |ALL| matches all ciphers, except for deprecated ciphers which must be
// named explicitly.
//
-// |kRSA|, |kDHE|, |kECDHE|, and |kPSK| match ciphers using plain RSA, DHE,
+// - |kRSA|, |kDHE|, |kECDHE|, and |kPSK| match ciphers using plain RSA, DHE,
// ECDHE, and plain PSK key exchanges, respectively. Note that ECDHE_PSK is
// matched by |kECDHE| and not |kPSK|.
//
-// |aRSA|, |aECDSA|, and |aPSK| match ciphers authenticated by RSA, ECDSA, and
+// - |aRSA|, |aECDSA|, and |aPSK| match ciphers authenticated by RSA, ECDSA, and
// a pre-shared key, respectively.
//
-// |RSA|, |DHE|, |ECDHE|, |PSK|, |ECDSA|, and |PSK| are aliases for the
+// - |RSA|, |DHE|, |ECDHE|, |PSK|, |ECDSA|, and |PSK| are aliases for the
// corresponding |k*| or |a*| cipher rule. |RSA| is an alias for |kRSA|, not
// |aRSA|.
//
-// |3DES|, |AES128|, |AES256|, |AES|, |AESGCM|, |CHACHA20| match ciphers
+// - |3DES|, |AES128|, |AES256|, |AES|, |AESGCM|, |CHACHA20| match ciphers
// whose bulk cipher use the corresponding encryption scheme. Note that
// |AES|, |AES128|, and |AES256| match both CBC and GCM ciphers.
//
-// |SHA1|, and its alias |SHA|, match legacy cipher suites using HMAC-SHA1.
+// - |SHA1|, and its alias |SHA|, match legacy cipher suites using HMAC-SHA1.
//
// Deprecated cipher rules:
//
-// |kEDH|, |EDH|, |kEECDH|, and |EECDH| are legacy aliases for |kDHE|, |DHE|,
+// - |kEDH|, |EDH|, |kEECDH|, and |EECDH| are legacy aliases for |kDHE|, |DHE|,
// |kECDHE|, and |ECDHE|, respectively.
//
-// |HIGH| is an alias for |ALL|.
+// - |HIGH| is an alias for |ALL|.
//
-// |FIPS| is an alias for |HIGH|.
+// - |FIPS| is an alias for |HIGH|.
//
-// |SSLv3| and |TLSv1| match ciphers available in TLS 1.1 or earlier.
+// - |SSLv3| and |TLSv1| match ciphers available in TLS 1.1 or earlier.
// |TLSv1_2| matches ciphers new in TLS 1.2. This is confusing and should not
// be used.
//
@@ -5319,7 +5319,7 @@
// doesn't undo other policies it's an error to try and set it.
ssl_compliance_policy_none,
- // ssl_policy_fips_202205 configures a TLS connection to use:
+ // ssl_compliance_policy_fips_202205 configures a TLS connection to use:
// * TLS 1.2 or 1.3
// * For TLS 1.2, only ECDHE_[RSA|ECDSA]_WITH_AES_*_GCM_SHA*.
// * For TLS 1.3, only AES-GCM
diff --git a/util/doc.css b/util/doc.css
index a868e44..f176f25 100644
--- a/util/doc.css
+++ b/util/doc.css
@@ -16,12 +16,13 @@
margin-bottom: 2em;
}
-ol {
+ol.toc {
list-style: none;
+ padding-left: 0;
margin-bottom: 4em;
}
-li a {
+ol.toc li a {
color: black;
}
@@ -49,12 +50,16 @@
font-size: 1.5em;
}
-.section pre {
+pre.code {
background-color: #b2c9db;
padding: 5px;
border-radius: 5px;
}
+.comment pre {
+ margin-left: 2em;
+}
+
td {
padding: 2px;
}
diff --git a/util/doc.go b/util/doc.go
index ae5c45d..21d9dab 100644
--- a/util/doc.go
+++ b/util/doc.go
@@ -18,7 +18,9 @@
"os"
"path/filepath"
"regexp"
+ "strconv"
"strings"
+ "unicode"
)
// Config describes the structure of the config JSON file.
@@ -41,7 +43,7 @@
Name string
// Preamble contains a comment for the file as a whole. Each string
// is a separate paragraph.
- Preamble []string
+ Preamble []CommentBlock
Sections []HeaderSection
// AllDecls maps all decls to their URL fragments.
AllDecls map[string]string
@@ -49,7 +51,7 @@
type HeaderSection struct {
// Preamble contains a comment for a group of functions.
- Preamble []string
+ Preamble []CommentBlock
Decls []HeaderDecl
// Anchor, if non-empty, is the URL fragment to use in anchor tags.
Anchor string
@@ -62,7 +64,7 @@
// Comment contains a comment for a specific function. Each string is a
// paragraph. Some paragraph may contain \n runes to indicate that they
// are preformatted.
- Comment []string
+ Comment []CommentBlock
// Name contains the name of the function, if it could be extracted.
Name string
// Decl contains the preformatted C declaration itself.
@@ -71,6 +73,20 @@
Anchor string
}
+type CommentBlockType int
+
+const (
+ CommentParagraph CommentBlockType = iota
+ CommentOrderedListItem
+ CommentBulletListItem
+ CommentCode
+)
+
+type CommentBlock struct {
+ Type CommentBlockType
+ Paragraph string
+}
+
const (
cppGuard = "#if defined(__cplusplus)"
commentStart = "/* "
@@ -95,7 +111,7 @@
return line[:idx]
}
-func extractComment(lines []string, lineNo int) (comment []string, rest []string, restLineNo int, err error) {
+func extractCommentLines(lines []string, lineNo int) (comment []string, rest []string, restLineNo int, err error) {
if len(lines) == 0 {
return nil, lines, lineNo, nil
}
@@ -109,22 +125,19 @@
} else if !strings.HasPrefix(rest[0], lineComment) {
panic("extractComment called on non-comment")
}
- commentParagraph := rest[0][len(commentStart):]
+ comment = []string{rest[0][len(commentStart):]}
rest = rest[1:]
restLineNo++
for len(rest) > 0 {
if isBlock {
- i := strings.Index(commentParagraph, commentEnd)
- if i >= 0 {
- if i != len(commentParagraph)-len(commentEnd) {
+ last := &comment[len(comment)-1]
+ if i := strings.Index(*last, commentEnd); i >= 0 {
+ if i != len(*last)-len(commentEnd) {
err = fmt.Errorf("garbage after comment end on line %d", restLineNo)
return
}
- commentParagraph = commentParagraph[:i]
- if len(commentParagraph) > 0 {
- comment = append(comment, commentParagraph)
- }
+ *last = (*last)[:i]
return
}
}
@@ -136,31 +149,9 @@
return
}
} else if !strings.HasPrefix(line, "//") {
- if len(commentParagraph) > 0 {
- comment = append(comment, commentParagraph)
- }
return
}
- if len(line) == 2 || !isBlock || line[2] != '/' {
- line = line[2:]
- }
- if strings.HasPrefix(line, " ") {
- /* Identing the lines of a paragraph marks them as
- * preformatted. */
- if len(commentParagraph) > 0 {
- commentParagraph += "\n"
- }
- line = line[3:]
- }
- if len(line) > 0 {
- commentParagraph = commentParagraph + line
- if len(commentParagraph) > 0 && commentParagraph[0] == ' ' {
- commentParagraph = commentParagraph[1:]
- }
- } else {
- comment = append(comment, commentParagraph)
- commentParagraph = ""
- }
+ comment = append(comment, line[2:])
rest = rest[1:]
restLineNo++
}
@@ -169,6 +160,128 @@
return
}
+func removeBulletListMarker(line string) (string, bool) {
+ orig := line
+ line = strings.TrimSpace(line)
+ if !strings.HasPrefix(line, "+ ") && !strings.HasPrefix(line, "- ") && !strings.HasPrefix(line, "* ") {
+ return orig, false
+ }
+ return line[2:], true
+}
+
+func removeOrderedListMarker(line string) (rest string, num int, ok bool) {
+ orig := line
+ line = strings.TrimSpace(line)
+ if len(line) == 0 || !unicode.IsDigit(rune(line[0])) {
+ return orig, -1, false
+ }
+
+ l := 0
+ for l < len(line) && unicode.IsDigit(rune(line[l])) {
+ l++
+ }
+ num, err := strconv.Atoi(line[:l])
+ if err != nil {
+ return orig, -1, false
+ }
+
+ line = line[l:]
+ if line, ok := strings.CutPrefix(line, ". "); ok {
+ return line, num, true
+ }
+ if line, ok := strings.CutPrefix(line, ") "); ok {
+ return line, num, true
+ }
+
+ return orig, -1, false
+}
+
+func removeCodeIndent(line string) (string, bool) {
+ return strings.CutPrefix(line, " ")
+}
+
+func extractComment(lines []string, lineNo int) (comment []CommentBlock, rest []string, restLineNo int, err error) {
+ commentLines, rest, restLineNo, err := extractCommentLines(lines, lineNo)
+ if err != nil {
+ return
+ }
+
+ // This syntax and parsing algorithm is loosely inspired by CommonMark,
+ // but reduced to a small subset with no nesting. Blocks being open vs.
+ // closed can be tracked implicitly. We're also much slopplier about how
+ // indentation. Additionally, rather than grouping list items into
+ // lists, our parser just emits a list items, which are grouped later at
+ // rendering time.
+ //
+ // If we later need more features, such as nested lists, this can evolve
+ // into a more complex implementation.
+ var numBlankLines int
+ for _, line := range commentLines {
+ // Defer blank lines until we know the next element.
+ if len(strings.TrimSpace(line)) == 0 {
+ numBlankLines++
+ continue
+ }
+
+ blankLinesSkipped := numBlankLines
+ numBlankLines = 0
+
+ // Attempt to continue the previous block.
+ if len(comment) > 0 {
+ last := &comment[len(comment)-1]
+ if last.Type == CommentCode {
+ l, ok := removeCodeIndent(line)
+ if ok {
+ for i := 0; i < blankLinesSkipped; i++ {
+ last.Paragraph += "\n"
+ }
+ last.Paragraph += l + "\n"
+ continue
+ }
+ } else if blankLinesSkipped == 0 {
+ _, isBulletList := removeBulletListMarker(line)
+ _, num, isOrderedList := removeOrderedListMarker(line)
+ if isOrderedList && last.Type == CommentParagraph && num != 1 {
+ // A list item can only interrupt a paragraph if the number is one.
+ // See the discussion in https://spec.commonmark.org/0.30/#lists.
+ // This avoids wrapping like "(See RFC\n5280)" turning into a list.
+ isOrderedList = false
+ }
+ if !isBulletList && !isOrderedList {
+ // This is a continuation line of the previous paragraph.
+ last.Paragraph += " " + strings.TrimSpace(line)
+ continue
+ }
+ }
+ }
+
+ // Make a new block.
+ if line, ok := removeBulletListMarker(line); ok {
+ comment = append(comment, CommentBlock{
+ Type: CommentBulletListItem,
+ Paragraph: strings.TrimSpace(line),
+ })
+ } else if line, _, ok := removeOrderedListMarker(line); ok {
+ comment = append(comment, CommentBlock{
+ Type: CommentOrderedListItem,
+ Paragraph: strings.TrimSpace(line),
+ })
+ } else if line, ok := removeCodeIndent(line); ok {
+ comment = append(comment, CommentBlock{
+ Type: CommentCode,
+ Paragraph: line + "\n",
+ })
+ } else {
+ comment = append(comment, CommentBlock{
+ Type: CommentParagraph,
+ Paragraph: strings.TrimSpace(line),
+ })
+ }
+ }
+
+ return
+}
+
func extractDecl(lines []string, lineNo int) (decl string, rest []string, restLineNo int, err error) {
if len(lines) == 0 || len(lines[0]) == 0 {
return "", lines, lineNo, nil
@@ -390,7 +503,8 @@
return nil, err
}
if len(rest) > 0 && len(rest[0]) == 0 {
- anchor := sanitizeAnchor(firstSentence(comment))
+ heading := firstSentence(comment)
+ anchor := sanitizeAnchor(heading)
if len(anchor) > 0 {
if _, ok := allAnchors[anchor]; ok {
return nil, fmt.Errorf("duplicate anchor: %s", anchor)
@@ -399,7 +513,7 @@
}
section.Preamble = comment
- section.IsPrivate = len(comment) > 0 && isPrivateSection(comment[0])
+ section.IsPrivate = isPrivateSection(heading)
section.Anchor = anchor
lines = rest[1:]
lineNo = restLineNo + 1
@@ -417,7 +531,7 @@
return nil, fmt.Errorf("hit ending C++ guard while in section on line %d (possibly missing two empty lines ahead of guard?)", lineNo)
}
- var comment []string
+ var comment []CommentBlock
var decl string
if isComment(line) {
comment, lines, lineNo, err = extractComment(lines, lineNo)
@@ -444,10 +558,11 @@
// with the name of the thing that they are
// commenting on. We make an exception here for
// collective comments.
+ sentence := firstSentence(comment)
if len(comment) > 0 &&
len(name) > 0 &&
- !isCollectiveComment(comment[0]) {
- subject := commentSubject(comment[0])
+ !isCollectiveComment(sentence) {
+ subject := commentSubject(sentence)
ok := subject == name
if l := len(subject); l > 0 && subject[l-1] == '*' {
// Groups of names, notably #defines, are often
@@ -486,11 +601,11 @@
return header, nil
}
-func firstSentence(paragraphs []string) string {
- if len(paragraphs) == 0 {
+func firstSentence(comment []CommentBlock) string {
+ if len(comment) == 0 {
return ""
}
- s := paragraphs[0]
+ s := comment[0].Paragraph
i := strings.Index(s, ". ")
if i >= 0 {
return s[:i]
@@ -501,6 +616,61 @@
return s
}
+func markupComment(allDecls map[string]string, comment []CommentBlock) template.HTML {
+ var b strings.Builder
+ lastType := CommentParagraph
+ closeList := func() {
+ if lastType == CommentOrderedListItem {
+ b.WriteString("</ol>")
+ } else if lastType == CommentBulletListItem {
+ b.WriteString("</ul>")
+ }
+ }
+
+ for _, block := range comment {
+ // Group consecutive list items of the same type into a list.
+ if block.Type != lastType {
+ closeList()
+ if block.Type == CommentOrderedListItem {
+ b.WriteString("<ol>")
+ } else if block.Type == CommentBulletListItem {
+ b.WriteString("<ul>")
+ }
+ }
+ lastType = block.Type
+
+ switch block.Type {
+ case CommentParagraph:
+ b.WriteString("<p>")
+ b.WriteString(string(markupParagraph(allDecls, block.Paragraph)))
+ b.WriteString("</p>")
+ case CommentOrderedListItem, CommentBulletListItem:
+ b.WriteString("<li>")
+ b.WriteString(string(markupParagraph(allDecls, block.Paragraph)))
+ b.WriteString("</li>")
+ case CommentCode:
+ b.WriteString("<pre>")
+ b.WriteString(block.Paragraph)
+ b.WriteString("</pre>")
+ default:
+ panic(block.Type)
+ }
+ }
+
+ closeList()
+ return template.HTML(b.String())
+}
+
+func markupParagraph(allDecls map[string]string, s string) template.HTML {
+ // TODO(davidben): Ideally the inline transforms would be unified into
+ // one pass, so that the HTML output of one pass does not interfere with
+ // the next.
+ ret := markupPipeWords(allDecls, s, true /* linkDecls */)
+ ret = markupFirstWord(ret)
+ ret = markupRFC(ret)
+ return ret
+}
+
// markupPipeWords converts |s| into an HTML string, safe to be included outside
// a tag, while also marking up words surrounded by |.
func markupPipeWords(allDecls map[string]string, s string, linkDecls bool) template.HTML {
@@ -585,27 +755,14 @@
return template.HTML(b.String())
}
-func newlinesToBR(html template.HTML) template.HTML {
- s := string(html)
- if !strings.Contains(s, "\n") {
- return html
- }
- s = strings.Replace(s, "\n", "<br>", -1)
- s = strings.Replace(s, " ", " ", -1)
- return template.HTML(s)
-}
-
func generate(outPath string, config *Config) (map[string]string, error) {
allDecls := make(map[string]string)
headerTmpl := template.New("headerTmpl")
headerTmpl.Funcs(template.FuncMap{
"firstSentence": firstSentence,
- "markupPipeWords": func(s string) template.HTML { return markupPipeWords(allDecls, s, true /* linkDecls */) },
"markupPipeWordsNoLink": func(s string) template.HTML { return markupPipeWords(allDecls, s, false /* linkDecls */) },
- "markupFirstWord": markupFirstWord,
- "markupRFC": markupRFC,
- "newlinesToBR": newlinesToBR,
+ "markupComment": func(c []CommentBlock) template.HTML { return markupComment(allDecls, c) },
})
headerTmpl, err := headerTmpl.Parse(`<!DOCTYPE html>
<html>
@@ -622,9 +779,9 @@
<a href="headers.html">All headers</a>
</div>
- {{range .Preamble}}<p>{{. | markupPipeWords | markupRFC}}</p>{{end}}
+ {{if .Preamble}}<div class="comment">{{.Preamble | markupComment}}</div>{{end}}
- <ol>
+ <ol class="toc">
{{range .Sections}}
{{if not .IsPrivate}}
{{if .Anchor}}<li class="header"><a href="#{{.Anchor}}">{{.Preamble | firstSentence | markupPipeWordsNoLink}}</a></li>{{end}}
@@ -638,18 +795,12 @@
{{range .Sections}}
{{if not .IsPrivate}}
<div class="section" {{if .Anchor}}id="{{.Anchor}}"{{end}}>
- {{if .Preamble}}
- <div class="sectionpreamble">
- {{range .Preamble}}<p>{{. | markupPipeWords | markupRFC}}</p>{{end}}
- </div>
- {{end}}
+ {{if .Preamble}}<div class="sectionpreamble comment">{{.Preamble | markupComment}}</div>{{end}}
{{range .Decls}}
<div class="decl" {{if .Anchor}}id="{{.Anchor}}"{{end}}>
- {{range .Comment}}
- <p>{{. | markupPipeWords | newlinesToBR | markupFirstWord | markupRFC}}</p>
- {{end}}
- {{if .Decl}}<pre>{{.Decl}}</pre>{{end}}
+ {{if .Comment}}<div class="comment">{{.Comment | markupComment}}</div>{{end}}
+ {{if .Decl}}<pre class="code">{{.Decl}}</pre>{{end}}
</div>
{{end}}
</div>
