Support @mnot's IETF Comments Markdown Format

ietf_reviewtool/boilerplate.py

@@ -30,13 +30,14 @@ def check_tlp(doc: Doc, review: IetfReview) -> None:
         )
         if doc.status.lower() == "standards track":
             msg += " And it cannot be published on the Standards Track."
-        review.discuss(msg)
+        review.discuss("Boilerplate", msg)
 
     if re.search(r"Simplified\s+BSD\s+License", text, flags=re.IGNORECASE):
         review.nit(
+            "Boilerplate",
             'Document still refers to the "Simplified BSD License", which '
             "was corrected in the TLP on September 21, 2021. It should "
-            'instead refer to the "Revised BSD License".'
+            'instead refer to the "Revised BSD License".',
         )
 
 
@@ -64,9 +65,10 @@ def check_boilerplate(doc: Doc, review: IetfReview) -> None:
 
         if doc.status.lower() in ["informational", "experimental"]:
             review.comment(
+                "Boilerplate",
                 f"Document has {doc.status} status, but uses the RFC2119 "
                 f"{kw_text} {used_keywords_str}. Check if this is really "
-                f"necessary?"
+                f"necessary?",
             )
 
         if not has_8174_boilerplate:
@@ -89,16 +91,17 @@ def check_boilerplate(doc: Doc, review: IetfReview) -> None:
                 msg += "text with a beginning similar to the RFC2119 boilerplate."
 
     if msg:
-        review.comment(msg)
+        review.comment("Boilerplate", msg)
 
     if uses_keywords:
         lc_not = set(re.findall(LC_NOT_KEYWORDS_PATTERN, doc.orig))
         if lc_not:
             lc_not_str = word_join(list(lc_not), prefix='"', suffix='"')
             review.comment(
+                "RFC2119 style",
                 f'Using lowercase "not" together with an uppercase '
                 f"RFC2119 keyword is not acceptable usage. Found: "
-                f"{lc_not_str}"
+                f"{lc_not_str}",
             )
 
     sotm = ""
@@ -122,8 +125,9 @@ def check_boilerplate(doc: Doc, review: IetfReview) -> None:
         sotm = re.sub(TLP_6A_PATTERN, r"", sotm)
     else:
         review.comment(
+            "Boilerplate",
             'TLP Section 6.a "Submission Compliance for '
-            'Internet-Drafts" boilerplate text seems to have issues.'
+            'Internet-Drafts" boilerplate text seems to have issues.',
         )
 
     idg_issues = False
@@ -134,42 +138,52 @@ def check_boilerplate(doc: Doc, review: IetfReview) -> None:
         elif required:
             idg_issues = True
     if idg_issues and not re.search(COPYRIGHT_ALT_STREAMS, sotm):
-        review.comment("I-D Guidelines boilerplate text seems to have issues.")
+        review.comment(
+            "Boilerplate",
+            "I-D Guidelines boilerplate text seems to have issues.",
+        )
 
     if re.search(COPYRIGHT_IETF, sotm):
         sotm = re.sub(COPYRIGHT_IETF, r"", sotm)
     elif re.search(COPYRIGHT_ALT_STREAMS, sotm):
         sotm = re.sub(COPYRIGHT_ALT_STREAMS, r"", sotm)
         review.comment(
-            'Document contains a TLP Section 6.b.ii "alternate streams" ' "boilerplate."
+            "Boilerplate",
+            'Document contains a TLP Section 6.b.ii "alternate streams" '
+            "boilerplate.",
         )
     else:
         review.comment(
+            "Boilerplate",
             'TLP Section 6.b "Copyright and License Notice" boilerplate '
-            "text seems to have issues."
+            "text seems to have issues.",
         )
 
     if re.search(NO_MOD_RFC, sotm):
         sotm = re.sub(NO_MOD_RFC, r"", sotm)
         review.comment(
+            "Boilerplate",
             "Document limits derivative works and/or RFC publication with "
             "a TLP Section 6.c.i boilerplate.",
         )
     elif re.search(NO_MOD_NO_RFC, sotm):
         sotm = re.sub(NO_MOD_NO_RFC, r"", sotm)
         review.comment(
+            "Boilerplate",
             "Document limits derivative works and/or RFC publication with "
             "a TLP Section 6.c.ii boilerplate.",
         )
     elif re.search(PRE_5378, sotm):
         sotm = re.sub(PRE_5378, r"", sotm)
         review.comment(
+            "Boilerplate",
             'Document has a TLP Section 6.c.iii "pre-5378" boilerplate. '
             "Is this really needed?",
         )
 
     if sotm:
         review.nit(
+            "Boilerplate",
             f'Found stray text in boilerplate: "{sotm.strip()}"',
         )
 

ietf_reviewtool/doc.py

@@ -7,7 +7,7 @@
 
 from .util.docposition import SECTION_PATTERN
 from .util.fetch import get_items, fetch_meta
-from .util.text import basename, unfold, untag, extract_urls
+from .util.text import basename, revision, unfold, untag, extract_urls
 from .util.utils import read
 
 
@@ -27,6 +27,7 @@ class Doc:
 
     def __init__(self, item: str, log: logging.Logger, datatracker: str):
         self.name = basename(item)
+        self.revision = revision(item)
         with tempfile.TemporaryDirectory() as tmp:
             current_directory = os.getcwd()
             log.debug("tmp dir %s", tmp)

ietf_reviewtool/grammar.py

@@ -7,13 +7,14 @@
 
 from .review import IetfReview
 from .util.docposition import DocPosition
-from .util.text import unfold
+from .util.text import unfold, wrap_para
 
 
 def check_grammar(
     text: str,
     grammar_skip_rules: str,
     review: IetfReview,
+    width: int,
     show_rule_id: bool = False,
 ) -> None:
     """
@@ -79,7 +80,7 @@ def check_grammar(
             pos += len(text[cur])
             cur += 1
 
-        review.nit(doc_pos.fmt_section_and_paragraph("nit"), end="")
+        nit = doc_pos.fmt_section_and_paragraph("nit")
         context = issue.context.lstrip(".")
         offset = issue.offsetInContext - (len(issue.context) - len(context))
         context = context.rstrip(".")
@@ -93,8 +94,8 @@ def check_grammar(
             context = context[cut:-cut]
             offset -= cut
 
-        review.nit("> " + context, wrap=False, end="")
-        review.nit("> " + " " * offset + "^" * issue.errorLength, wrap=False, end="")
+        nit += f"> {context}\n"
+        nit += f"> {' ' * offset}{'^' * issue.errorLength}\n"
 
         message = (
             issue.message.replace("“", '"')
@@ -111,4 +112,4 @@ def check_grammar(
         if show_rule_id:
             message = f"{message} [{issue.ruleId}]"
 
-        review.nit(message)
+        review.nit("Grammar/style", nit + wrap_para(message, "\n", width), wrap=False)

ietf_reviewtool/ietf_reviewtool.py

@@ -319,11 +319,10 @@ def thank_art_reviewer(
             continue
 
         if group["acronym"].lower() == thank_art.lower():
-            review.comment(
-                "Thanks to "
-                + (reviewer["name_from_draft"] or reviewer["name"])
-                + f" for their {group['name']} review "
-                f"({art_review['external_url']})."
+            review.preface(
+                "",
+                f'Thanks to {reviewer["name_from_draft"] or reviewer["name"]}'
+                + f'for the {group["name"]} review ({art_review["external_url"]}).',
             )
 
 
@@ -349,7 +348,7 @@ def check_ips(doc: Doc, review: IetfReview) -> None:
         else:
             msg += "block or address: "
         msg += word_join(faulty, prefix='"', suffix='"') + "."
-        review.nit(msg)
+        review.nit("IP addresses", msg)
 
     faulty = []
     for ip_obj in result:
@@ -385,7 +384,7 @@ def check_ips(doc: Doc, review: IetfReview) -> None:
             msg += "block or address"
         msg += " not inside RFC5737/RFC3849 example ranges: "
         msg += word_join(faulty, prefix='"', suffix='"') + "."
-        review.comment(msg)
+        review.comment("IP addresses", msg)
 
 
 def check_html_entities(doc: Doc, review: IetfReview) -> None:
@@ -406,9 +405,9 @@ def check_html_entities(doc: Doc, review: IetfReview) -> None:
 
         if entities:
             review.nit(
-                "The text version of this document contains "
-                "these HTML entities, which might indicate "
-                "issues with its XML source: "
+                "Stray characters",
+                "The text version of this document contains these HTML entities, "
+                "which might indicate issues with its XML source: "
                 f"{word_join(list(set(entities)))}",
             )
 
@@ -423,6 +422,7 @@ def check_urls(doc: Doc, review: IetfReview, verbose: bool) -> None:
 
     if result:
         review.nit_bullets(
+            "URLs",
             "These URLs point to tools.ietf.org, which is being deprecated:",
             result,
         )
@@ -434,7 +434,7 @@ def check_urls(doc: Doc, review: IetfReview, verbose: bool) -> None:
             result.append(url)
 
     if result:
-        review.nit_bullets("Found non-HTTP URLs in the document:", result)
+        review.nit_bullets("URLs", "Found non-HTTP URLs in the document:", result)
 
     reachability = {u: fetch_url(u, log, verbose, "HEAD") for u in urls}
     result = []
@@ -443,7 +443,9 @@ def check_urls(doc: Doc, review: IetfReview, verbose: bool) -> None:
             result.append(url)
 
     if result:
-        review.nit_bullets("These URLs in the document did not return content:", result)
+        review.nit_bullets(
+            "URLs", "These URLs in the document did not return content:", result
+        )
 
     result = []
     for url in urls:
@@ -456,6 +458,7 @@ def check_urls(doc: Doc, review: IetfReview, verbose: bool) -> None:
 
     if result:
         review.nit_bullets(
+            "URLs",
             "These URLs in the document can probably be converted " "to HTTPS:",
             result,
         )
@@ -492,7 +495,25 @@ def check_xml(doc: Doc, review: IetfReview) -> None:
             xml.etree.ElementTree.fromstring(text)
         except xml.etree.ElementTree.ParseError as err:
             print(text[err.position[0] - 2])
-            review.nit(f'XML issue: "{err}":\n> {text[err.position[0] - 2]}')
+            review.nit("XML", f'XML issue: "{err}":\n> {text[err.position[0] - 2]}')
+
+
+def validate_gh_id(_ctx, _param, value):
+    """
+    Validate the --github-id parameter. See
+    https://click.palletsprojects.com/en/8.1.x/options/#callbacks-for-validation
+
+    @param      _ctx    The context
+    @param      _param  The parameter
+    @param      value   The value
+
+    @return     The value
+    """
+    if isinstance(value, tuple):
+        return value
+    if not re.match(r"@\w+", value):
+        raise click.BadParameter("GitHub user ID must be in the form '@username'")
+    return value
 
 
 @click.command("review", help="Extract review from named items.")
@@ -564,6 +585,28 @@ def check_xml(doc: Doc, review: IetfReview) -> None:
     default="genart",
     help="Generate a thank-you for the given Area Review Team reviewer.",
 )
+@click.option(
+    "--output-markdown",
+    "gen_md",
+    is_flag=True,
+    help=(
+        "Generate review in IETF Comments Markdown Format. "
+        "See https://github.com/mnot/ietf-comments/blob/main/format.md"
+    ),
+)
+@click.option(
+    "--role",
+    "role",
+    default="",
+    help="Indicate the role you are reviewing this document in (if any).",
+)
+@click.option(
+    "--github-id",
+    "gh_id",
+    default="",
+    callback=validate_gh_id,
+    help='Your GitHub ID ("@username").',
+)
 @click.pass_obj
 def review_items(
     state: State,
@@ -579,6 +622,9 @@ def review_items(
     chk_tlp: bool,
     thank_art: str,
     grammar_skip_rules: str,
+    gen_md: bool,
+    role: str,
+    gh_id: str,
 ) -> None:
     """
     Extract reviews from named items.
@@ -616,7 +662,7 @@ def review_items(
             continue
 
         doc = Doc(item, log, state.datatracker)
-        review = IetfReview(doc, state.width)
+        review = IetfReview(doc, gen_md, role.strip(), gh_id.strip(), state.width)
 
         if chk_misc:
             check_html_entities(doc, review)
@@ -649,10 +695,19 @@ def review_items(
         thank_art_reviewer(doc, review, thank_art, state.datatracker)
 
         if doc.name.startswith("charter-"):
-            review.comment("Note to self: Ask about any chair changes.")
+            review.comment("Note to self", "Ask about any chair changes.")
 
         if chk_grammar:
-            check_grammar(doc.current, grammar_skip_rules, review, verbose)
+            check_grammar(doc.current, grammar_skip_rules, review, state.width, verbose)
+
+        if gen_md:
+            review.note(
+                "",
+                'This review is formatted in the "IETF Comments" Markdown format, '
+                "see https://github.com/mnot/ietf-comments. Generated by the "
+                '"IETF Review Tool", see '
+                "https://github.com/larseggert/ietf-reviewtool.",
+            )
 
         print(review)
 

ietf_reviewtool/inclusive.py

@@ -67,4 +67,4 @@ def check_inclusivity(
             if verbose:
                 msg += f' (matched "{name}" rule, pattern {match[1]})'
             comment_items.append(msg)
-        review.comment_bullets(comment_header, comment_items)
+        review.comment_bullets("Inclusive language", comment_header, comment_items)