netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Donald Hunter <donald.hunter@gmail.com>
To: Breno Leitao <leitao@debian.org>
Cc: corbet@lwn.net,  linux-doc@vger.kernel.org,
	 netdev@vger.kernel.org, kuba@kernel.org,  pabeni@redhat.com,
	 edumazet@google.com
Subject: Re: [PATCH] Documentation: Document the Netlink spec
Date: Wed, 08 Nov 2023 14:03:34 +0000	[thread overview]
Message-ID: <m2y1f8mjex.fsf@gmail.com> (raw)
In-Reply-To: <20231103135622.250314-1-leitao@debian.org> (Breno Leitao's message of "Fri, 3 Nov 2023 06:56:22 -0700")

Breno Leitao <leitao@debian.org> writes:

> This is a Sphinx extension that parses the Netlink YAML spec files
> (Documentation/netlink/specs/), and generates a rst file to be
> displayed into Documentation pages.
>
> Create a new Documentation/networking/netlink_spec page, and a sub-page
> for each Netlink spec that needs to be documented, such as ethtool,
> devlink, netdev, etc.
>
> Create a Sphinx directive extension that reads the YAML spec
> (located under Documentation/netlink/specs), parses it and returns a RST
> string that is inserted where the Sphinx directive was called.

This is great! Looks like I need to fill in some missing docs in the
specs I have contributed.

I wonder if the generated .rst content can be adjusted to improve the
resulting HTML.

There are a couple of places where paragraph text is indented and I
don't think it needs to be, e.g. the 'Summary' doc.

A lot of the .rst content seems to be over-indented which causes
blockquote tags to be generated in the HTML. That combined with a
mixture of bullets and definition lists at the same indentation level
seems to produce HTML with inconsistent indentation.

I quickly hacked the diff below to see if it would improve the HTML
rendering. I think the HTML has fewer odd constructs and the indentation
seems better to my eye. My main aim was to ensure that for a given
section, each indentation level uses the same construct, whether it be a
definition list or a bullet list.

It would be great to generate links from e.g. an attribute-set to its
definition.

Did you intentionally leave out the protocol values?

It looks like parse_entries will need to be extended to include the type
information for struct members, similar to how attribute sets are shown.
I'd be happy to look at this as a follow up patch, unless you get there
first. 

Thanks,
Donald.

diff --git a/Documentation/sphinx/netlink_spec.py b/Documentation/sphinx/netlink_spec.py
index 80756e72ed4f..66ba9106b4ea 100755
--- a/Documentation/sphinx/netlink_spec.py
+++ b/Documentation/sphinx/netlink_spec.py
@@ -92,7 +92,7 @@ def parse_mcast_group(mcast_group: List[Dict[str, Any]]) -> str:
     """Parse 'multicast' group list and return a formatted string"""
     lines = []
     for group in mcast_group:
-        lines.append(rst_paragraph(group["name"], 1))
+        lines.append(rst_bullet(group["name"]))
 
     return "\n".join(lines)
 
@@ -101,7 +101,7 @@ def parse_do(do_dict: Dict[str, Any], level: int = 0) -> str:
     """Parse 'do' section and return a formatted string"""
     lines = []
     for key in do_dict.keys():
-        lines.append(rst_bullet(bold(key), level + 1))
+        lines.append("     " + bold(key))
         lines.append(parse_do_attributes(do_dict[key], level + 1) + "\n")
 
     return "\n".join(lines)
@@ -124,18 +124,19 @@ def parse_operations(operations: List[Dict[str, Any]]) -> str:
     for operation in operations:
         lines.append(rst_subsubtitle(operation["name"]))
         lines.append(rst_paragraph(operation["doc"]) + "\n")
-        if "do" in operation:
-            lines.append(rst_paragraph(bold("do"), 1))
-            lines.append(parse_do(operation["do"], 1))
-        if "dump" in operation:
-            lines.append(rst_paragraph(bold("dump"), 1))
-            lines.append(parse_do(operation["dump"], 1))
 
         for key in operation.keys():
             if key in preprocessed:
                 # Skip the special fields
                 continue
-            lines.append(rst_fields(key, operation[key], 1))
+            lines.append(rst_fields(key, operation[key], 0))
+
+        if "do" in operation:
+            lines.append(rst_paragraph(":do:", 0))
+            lines.append(parse_do(operation["do"], 0))
+        if "dump" in operation:
+            lines.append(rst_paragraph(":dump:", 0))
+            lines.append(parse_do(operation["dump"], 0))
 
         # New line after fields
         lines.append("\n")
@@ -150,7 +151,7 @@ def parse_entries(entries: List[Dict[str, Any]], level: int) -> str:
         if isinstance(entry, dict):
             # entries could be a list or a dictionary
             lines.append(
-                rst_fields(entry.get("name"), sanitize(entry.get("doc")), level)
+                rst_fields(entry.get("name"), sanitize(entry.get("doc") or ""), level)
             )
         elif isinstance(entry, list):
             lines.append(rst_list_inline(entry, level))
@@ -172,16 +173,16 @@ def parse_definitions(defs: Dict[str, Any]) -> str:
         for k in definition.keys():
             if k in preprocessed + ignored:
                 continue
-            lines.append(rst_fields(k, sanitize(definition[k]), 1))
+            lines.append(rst_fields(k, sanitize(definition[k]), 0))
 
         # Field list needs to finish with a new line
         lines.append("\n")
         if "entries" in definition:
-            lines.append(rst_paragraph(bold("Entries"), 1))
-            lines.append(parse_entries(definition["entries"], 2))
+            lines.append(rst_paragraph(":entries:", 0))
+            lines.append(parse_entries(definition["entries"], 1))
         if "members" in definition:
-            lines.append(rst_paragraph(bold("members"), 1))
-            lines.append(parse_entries(definition["members"], 2))
+            lines.append(rst_paragraph(":members:", 0))
+            lines.append(parse_entries(definition["members"], 1))
 
     return "\n".join(lines)
 
@@ -201,12 +202,12 @@ def parse_attributes_set(entries: List[Dict[str, Any]]) -> str:
                 # Add the attribute type in the same line
                 attr_line += f" ({inline(type_)})"
 
-            lines.append(rst_bullet(attr_line, 2))
+            lines.append(rst_bullet(attr_line, 1))

             for k in attr.keys():
                 if k in preprocessed + ignored:
                     continue
-                lines.append(rst_fields(k, sanitize(attr[k]), 3))
+                lines.append(rst_fields(k, sanitize(attr[k]), 2))
             lines.append("\n")
 
     return "\n".join(lines)
@@ -218,7 +219,7 @@ def parse_yaml(obj: Dict[str, Any]) -> str:
 
     # This is coming from the RST
     lines.append(rst_subtitle("Summary"))
-    lines.append(rst_paragraph(obj["doc"], 1))
+    lines.append(rst_paragraph(obj["doc"], 0))
 
     # Operations
     lines.append(rst_subtitle("Operations"))

  parent reply	other threads:[~2023-11-08 14:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-03 13:56 [PATCH] Documentation: Document the Netlink spec Breno Leitao
2023-11-06 22:51 ` Jakub Kicinski
2023-11-08 14:03 ` Donald Hunter [this message]
2023-11-08 14:08   ` Donald Hunter
2023-11-09 18:28   ` Breno Leitao
2023-11-08 20:27 ` Jonathan Corbet
2023-11-09  1:43   ` Jakub Kicinski
2023-11-10  9:23     ` Donald Hunter
2023-11-09 11:22   ` Donald Hunter
2023-11-09 14:12     ` Jonathan Corbet
2023-11-09 15:16       ` Jakub Kicinski
2023-11-09 15:20       ` Breno Leitao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2y1f8mjex.fsf@gmail.com \
    --to=donald.hunter@gmail.com \
    --cc=corbet@lwn.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).