Tighten code comments in src/content-render/unified (#63437)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7c52b57f-2c29-4aa1-8233-99d0d6e13571

Author: heiskr

src/content-render/unified/README.md

@@ -0,0 +1,23 @@
+# Unified content rendering notes
+
+## Annotated code blocks
+
+The `annotate` plugin parses fenced code blocks whose info string includes `annotate`. It splits the rendered output into `.annotate-row` elements, with code in `.annotate-code` and rendered notes in `.annotate-note`.
+
+Authoring rules:
+
+- Include `annotate` in the info string.
+- Include a language on the opening code fence.
+- Start notes with the single-line comment marker for the fenced language: `#`, `//`, `<!--`, `%%`, or `--`.
+- Match the comment marker style to the code fence language.
+- Use single-line comments only. Multiline comment syntax is not supported.
+- Put a space between the comment marker and annotation text.
+- Leave text after the comment marker blank to create a blank annotation.
+- Do not create a blank code block.
+- Put Markdown after the comment marker. Inline Markdown is supported. Avoid block Markdown such as headings, blockquotes, horizontal rules, tables, lists, or code fences.
+- Consecutive lines with the comment marker become one annotation.
+- Empty lines and lines that contain only spaces are discarded.
+- Start the code section with a single-line comment, or rendering throws.
+- For HTML fences, add a line such as `<!-- -->` after the annotations to keep syntax highlighting.
+
+`parse-info-string.ts` must run before `remark-rehype`, and `annotate` must run before `highlight`.

src/content-render/unified/alerts.ts

@@ -1,6 +1,4 @@
-/*
-Custom "Alerts", based on similar filter/styling in the monolith code.
-*/
+// Matches the monolith alert syntax and styling.
 
 import { visit } from 'unist-util-visit'
 import { h } from 'hastscript'
@@ -23,7 +21,7 @@ const alertTypes: Record<string, AlertType> = {
   CAUTION: { icon: 'stop', color: 'danger' },
 }
 
-// Must contain one of [!NOTE], [!IMPORTANT], ...
+// Matches alert markers such as [!NOTE] and [!IMPORTANT].
 const ALERT_REGEXP = new RegExp(`\\[!(${Object.keys(alertTypes).join('|')})\\]`, 'gi')
 // Non-global version for .test() and .match() to avoid stateful lastIndex issues
 const ALERT_REGEXP_DETECT = new RegExp(`\\[!(${Object.keys(alertTypes).join('|')})\\]`, 'i')

src/content-render/unified/annotate.ts

@@ -1,32 +1,4 @@
-/*
-Parses fenced code blocks with `annotate` in info string.
-Results in single line comments split out, output format is:
-
-.annotate
-  .annotate-row (n)
-    .annotate-code
-    .annotate-note
-
-Contributing rules:
-- You must include `annotate` in the info string
-- You must include a language on the starting ` ``` ` tag.
-- Notes must start with one of: `#`, `//`, `<!--`, `%%`. (comment tag)
-- The comment tag style must match the language on the code fence.
-- Multiline-style comments, such as `/*` are not supported.
-- You can include any number of spaces before the comment tag starts.
-- You can include any number of spaces after the comment tag ends.
-- You can leave after the comment tag blank to create a blank annotation.
-- You cannot create a blank code block however.
-- Anything after the comment tag will be parsed with Markdown.
-- You can use any inline Markdown tag in the comment; recommend against using block tags such as headings, blockquote, horizontal rules, tables, lists, or code fences.
-- Multiple lines in row with the comment tag will result in a single annotation.
-- Empty lines, or lines that contain only space characters, will be discarded.
-- You must start the code section with a single line comment, otherwise the two will be flipped.
-- For HTML style, you can include a line after your annotations such as `<!-- -->` to maintain syntax highlighting; this will not impact what renders.
-
-`parse-info-string.ts` plugin is required for this to work, and must come before `remark-rehype`.
-`annotate` must come before the `highlight` plugin.
-*/
+// Annotate fences split single-line comments into rendered notes beside code examples.
 
 import { load } from 'js-yaml'
 import fs from 'fs'
@@ -89,22 +61,7 @@ const languages = load(fs.readFileSync('./data/code-languages.yml', 'utf8')) as
 >
 
 const commentRegexes = {
-  // Also known has hash or sharp; but the unicode name is "number sign".
-  // The reason this has 2 variants is because the hash is used, in bash
-  // for both hash-hang and for comments.
-  // For example:
-  //
-  //     #!/bin/bash
-  //
-  // ...is not a comment.
-  // But if you only look for `#` followed by anything-but `!` it will not
-  // match if the line is just `#`.
-  //
-  //    > /^\s*#[^!]\s*/.test('#')
-  //    false
-  //
-  // Which makes sense, because the `#` is not followed by anything.
-  // That's why we use the | operator to make an "exception" for that case.
+  // Keep shebang lines in code, but treat a line that only contains a number sign as a comment.
   number: /^\s*#[^!]\s*|^\s*#$/,
   slash: /^\s*\/\/\s*/,
   xml: /^\s*<!--\s*/,
@@ -286,10 +243,10 @@ function processAutotitleInMdast(mdast: Root, context: Context): void {
           const page = findPage(node.url, context.pages, context.redirects)
           if (page) {
             try {
-              // Use rawTitle for synchronous processing in annotations
+              // rawTitle avoids async title rendering while annotation notes convert to HAST.
               child.value = page.rawTitle || 'AUTOTITLE'
             } catch (error) {
-              // Keep AUTOTITLE if we can't get the title
+              // Keep AUTOTITLE when title lookup fails.
               logger.warn('Could not resolve AUTOTITLE', {
                 url: node.url,
                 error: error instanceof Error ? error.message : String(error),
@@ -308,7 +265,6 @@ function removeComment(lang: string): (line: string) => string {
 }
 
 function getPreMeta(node: ElementNode): { annotate?: boolean; [key: string]: unknown } {
-  // Here's why this monstrosity works:
-  // https://github.com/syntax-tree/mdast-util-to-hast/blob/c87cd606731c88a27dbce4bfeaab913a9589bf83/lib/handlers/code.js#L40-L42
+  // mdast-util-to-hast stores code-fence metadata on the code child.
   return node.children[0]?.data?.meta || {}
 }

src/content-render/unified/code-header.ts

@@ -1,8 +1,3 @@
-/**
- * Adds a bar above code blocks that shows the language and a copy button.
- * Optionally, adds a prompt button to Copilot Chat blocks.
- */
-
 import { load } from 'js-yaml'
 import fs from 'fs'
 import { visit } from 'unist-util-visit'
@@ -51,7 +46,7 @@ function wrapCodeExample(node: Element, tree: Root): Element {
   const subnav = null // getSubnav() lives in annotate.ts, not needed for normal code blocks
   const hasPrompt: boolean = Boolean(getPreMeta(node).prompt)
   const promptResult = hasPrompt ? getPrompt(node, tree, code) : null
-  const hasCopy: boolean = Boolean(getPreMeta(node).copy) // defaults to true
+  const hasCopy: boolean = Boolean(getPreMeta(node).copy)
 
   const headerHast = header(
     lang,
@@ -120,10 +115,9 @@ function btnIcon(): Element {
   return btnIconElement as Element
 }
 
-// node can be various hast element types, return value contains meta properties from code blocks
+// mdast-util-to-hast stores code-fence metadata on the first child.
 export function getPreMeta(node: Element): Record<string, unknown> {
-  // Here's why this monstrosity works:
-  // https://github.com/syntax-tree/mdast-util-to-hast/blob/c87cd606731c88a27dbce4bfeaab913a9589bf83/lib/handlers/code.js#L40-L42
+  // Code block callers pass pre elements whose code child owns the metadata.
   const firstChild = node.children[0] as Element | undefined
   return (firstChild?.data as Record<string, Record<string, unknown>> | undefined)?.meta || {}
 }

src/content-render/unified/collect-mini-toc.ts

@@ -22,9 +22,8 @@ function getClassString(el: Element): string {
   return ''
 }
 
-// Rehype plugin that collects heading data (href, title, level, platform)
-// during rendering, so callers don't need to re-parse the HTML.
-// Place this after heading-links in the processor chain.
+// Collect heading href, title, level, and platform during rendering, so callers
+// do not re-parse HTML. Run after heading-links so anchors exist.
 const collectMiniToc: Plugin<[CollectMiniTocOptions], Root> = ({ collectInto }) => {
   if (!collectInto) return
 
@@ -34,7 +33,7 @@ const collectMiniToc: Plugin<[CollectMiniTocOptions], Root> = ({ collectInto })
       if (!/^h[1-6]$/.test(el.tagName)) return
       if (!el.properties?.id) return
 
-      // Skip headings inside hidden ancestors
+      // Hidden headings must not appear in the mini TOC.
       for (const anc of ancestors) {
         if (anc.type === 'element') {
           const ancEl = anc as Element
@@ -44,7 +43,7 @@ const collectMiniToc: Plugin<[CollectMiniTocOptions], Root> = ({ collectInto })
 
       const headingLevel = parseInt(el.tagName.charAt(1), 10)
 
-      // Find the anchor child that heading-links.ts created
+      // heading-links.ts creates the anchor child that owns the rendered heading text.
       const anchor = el.children.find(
         (child): child is Element =>
           child.type === 'element' && child.tagName === 'a' && hasClassName(child, 'heading-link'),
@@ -54,9 +53,7 @@ const collectMiniToc: Plugin<[CollectMiniTocOptions], Root> = ({ collectInto })
       const href = anchor.properties?.href as string | undefined
       if (!href) return
 
-      // Filter out direct-child <span> elements (and their content).
-      // heading-links.ts always inserts the heading-link-symbol span as a
-      // direct child of the anchor, so filtering direct children is sufficient.
+      // heading-links.ts inserts heading-link-symbol directly, so direct filtering is enough.
       const textChildren = (anchor.children || []).filter(
         (child: ElementContent) =>
           !(child.type === 'element' && (child as Element).tagName === 'span'),

src/content-render/unified/copilot-prompt.ts

@@ -1,7 +1,3 @@
-/**
- * Adds a runnable prompt button in the header of Copilot Chat blocks.
- */
-
 import { find } from 'unist-util-find'
 import { h } from 'hastscript'
 import octicons from '@primer/octicons'
@@ -24,7 +20,7 @@ export function getPrompt(
 
   const { promptContent, ariaLabel } = buildPromptData(node, tree, code)
   const promptLink = `https://github.com/copilot?prompt=${encodeURIComponent(promptContent.trim())}`
-  // Use murmur hash for deterministic ID (avoids hydration mismatch)
+  // Murmur keeps the prompt ID deterministic and avoids hydration mismatches.
   const promptId: string = generatePromptId(promptContent)
 
   const element = h(
@@ -50,17 +46,17 @@ function buildPromptData(
   const ref = getPreMeta(node).ref
 
   if (!ref) {
-    // If no 'ref=<id>' meta is found, use just the current code for the prompt link.
+    // Without ref metadata, the prompt has no extra code context.
     return promptOnly(code)
   }
 
-  // If the 'ref=<id>' meta is found, find a matching code block to include as context in the prompt link.
+  // ref metadata points to a code block that becomes prompt context.
   const matchingCodeEl = findMatchingCode(ref as string, tree)
   if (!matchingCodeEl) {
     logger.warn('Cannot find referenced code block', { ref })
     return promptOnly(code)
   }
-  // AST structure: element -> code -> text node with value property
+  // HAST nests fenced code text at element, code, then text.
   const codeChild = matchingCodeEl.children[0] as Element | undefined
   const textNode = codeChild?.children[0] as { value?: string } | undefined
   const matchingCode = textNode?.value || null

src/content-render/unified/index.ts

@@ -24,17 +24,8 @@ export async function renderUnified(
   return html.trim()
 }
 
-/**
- * Run the same unified pipeline as `renderUnified`, but stop before the terminal
- * rehype-stringify step and return the hast (HTML AST) tree instead of a string.
- *
- * The 18 transform plugins all run during `processor.run()`; rehype-stringify is
- * only invoked by `processor.stringify()`. So we get the fully transformed hast
- * from a single pass, then derive the legacy HTML string from that exact same
- * tree. This guarantees the string and hast can never drift, and avoids running
- * the pipeline twice for consumers that still need the string (mini-TOC text,
- * search indexing, validators).
- */
+// renderUnifiedToHast derives HTML from the same transformed HAST tree it returns,
+// so string and tree consumers cannot drift or run the pipeline twice.
 export async function renderUnifiedToHast(
   template: string,
   context: Context,
@@ -43,18 +34,12 @@ export async function renderUnifiedToHast(
   const mdast = processor.parse(template)
   const hast = (await processor.run(mdast)) as HastNodes
   const html = processor.stringify(hast).toString()
-  // Strip `position` data (line/column offsets) the parser leaves on every node.
-  // It is useless to the client and meaningfully inflates the serialized tree we
-  // ship through the Next props boundary. Done after deriving `html` so the
-  // string output is byte-identical to the legacy path.
+  // Strip positions after stringifying to shrink Next props without changing the HTML output.
   stripPositions(hast)
   return { hast: hast as HastRoot, html: html.trim() }
 }
 
-/**
- * Recursively delete `position` fields from a hast/unist tree in place.
- * Avoids adding a dependency (unist-util-remove-position) for one field.
- */
+// Delete position fields in place to avoid a dependency for one field.
 function stripPositions(node: HastNodes): void {
   if (node && typeof node === 'object') {
     if ('position' in node) delete (node as { position?: unknown }).position

src/content-render/unified/parse-info-string.ts

@@ -1,10 +1,8 @@
-// Based on https://spec.commonmark.org/0.30/#info-string
-// Parse out info strings on fenced code blocks, example:
-// ```javascript lineNumbers:left copy:all annotate
-// becomes...
-// node.lang = javascript
-// node.meta = { lineNumbers: 'left', copy: 'all', annotate: true }
-// Also parse equals signs, where id=some-id becomes { id: 'some-id' }
+// CommonMark defines fenced-code info strings: https://spec.commonmark.org/0.30/#info-string
+// Colon and equals metadata extend that grammar before rehype reads code data.
+// javascript lineNumbers:left copy:all annotate sets node.lang to javascript.
+// It sets node.meta to { lineNumbers: 'left', copy: 'all', annotate: true }.
+// id=some-id becomes { id: 'some-id' }.
 
 import { visit } from 'unist-util-visit'
 import type { Node } from 'unist'
@@ -24,7 +22,7 @@ export default function parseInfoString() {
       if (!matcher(node)) return
       node.meta = strToObj(node.meta as string)
 
-      // Temporary, remove {:copy} to avoid highlight parse error in translations.
+      // Translated code fences can include {:copy}, which highlight treats as part of the language.
       if (node.lang) {
         node.lang = node.lang.replace('{:copy}', '')
       }
@@ -37,7 +35,7 @@ function strToObj(str?: string): Record<string, string | boolean> {
   return Object.fromEntries(
     str
       .split(/\s+/g)
-      .map((k: string) => k.split(/[:=]/)) // split by colon or equals sign
+      .map((k: string) => k.split(/[:=]/))
       .map(([k, ...v]: string[]) => [k, v.length ? v.join(':') : true]),
   )
 }

src/content-render/unified/processor.ts

@@ -41,38 +41,24 @@ export function createProcessor(context: Context): UnifiedProcessor {
       .use(remarkParse)
       .use(removeHtmlComments)
       .use(gfm)
-      // Markdown AST below vvv
       .use(parseInfoString)
-      // Using type assertion because rewriteLocalLinks is a factory function that takes context
-      // and returns a transformer, but TypeScript's unified plugin types don't handle this pattern
+      // TypeScript's unified plugin types do not model context-bound transformer factories.
       .use(rewriteLocalLinks as unknown as (ctx: Context) => void, context)
       .use(emoji)
-      // Markdown AST above ^^^
       .use(remark2rehype, { allowDangerousHtml: true })
-      // HTML AST below vvv
       .use(slug)
-      // useEnglishHeadings plugin requires context with englishHeadings property
       .use(useEnglishHeadings as unknown as (ctx: Context) => void, context || {})
       .use(headingLinks)
       .use(codeHeader)
       .use(annotate as unknown as (ctx: Context) => void, context)
-      // Using type assertion for highlight plugin due to complex type mismatch between unified and rehype-highlight
+      // TypeScript's unified plugin types do not model rehype-highlight's lowlight options.
       .use(highlight as unknown as (options: unknown) => void, {
         languages: { ...common, graphql, dockerfile, http, groovy, erb, powershell },
         subset: false,
         aliases: {
-          // As of Jan 2024, 'jsonc' is not supported by highlight.js. It
-          // just becomes plain text.
-          // But 'jsonc' works great in github.com. For example, when
-          // previewing and edited .md content in the browser. Or viewing
-          // PR diffs in web view.
-          // So by sticking to 'jsonc' where there's JSON with comments,
-          // it's technically more correct, looks good in github.com,
-          // but with this alias you get the nice syntax highlighting when
-          // viewed on our site.
+          // Register jsonc as a JSON alias so JSON with comments gets docs-site highlighting.
           json: 'jsonc',
-          // Docs supports a custom 'copilot' language, which is useful for contributors,
-          // but is not a supported highlight.js language, so alias to 'text'.
+          // Map docs-only copilot fences to text so contributors can use them.
           text: 'copilot',
         },
       })
@@ -88,10 +74,8 @@ export function createProcessor(context: Context): UnifiedProcessor {
       .use(wrapCodeTerms)
       .use(rewriteImgSources)
       .use(rewriteAssetImgTags)
-      // alerts plugin requires context with alertTitles property
       .use(alerts as unknown as (ctx: Context) => void, context || {})
-      // HTML AST above ^^^
-      .use(html) as unknown as UnifiedProcessor // String below vvv
+      .use(html) as unknown as UnifiedProcessor
   )
 }
 
@@ -100,8 +84,7 @@ export function createMarkdownOnlyProcessor(context: Context): UnifiedProcessor
     unified()
       .use(remarkParse)
       .use(gfm)
-      // Using type assertion because rewriteLocalLinks is a factory function that takes context
-      // and returns a transformer, but TypeScript's unified plugin types don't handle this pattern
+      // TypeScript's unified plugin types do not model context-bound transformer factories.
       .use(rewriteLocalLinks as unknown as (ctx: Context) => void, context)
       .use(remarkStringify) as unknown as UnifiedProcessor
   )
@@ -112,8 +95,7 @@ export function createMinimalProcessor(context: Context): UnifiedProcessor {
     unified()
       .use(remarkParse)
       .use(gfm)
-      // Using type assertion because rewriteLocalLinks is a factory function that takes context
-      // and returns a transformer, but TypeScript's unified plugin types don't handle this pattern
+      // TypeScript's unified plugin types do not model context-bound transformer factories.
       .use(rewriteLocalLinks as unknown as (ctx: Context) => void, context)
       .use(remark2rehype, { allowDangerousHtml: true })
       .use(slug)