Skip to content

Convert Accumulators documentation from QuickBook to AsciiDoc/Antora#1

Open
myzhang-1127 wants to merge 6 commits intoCppDigest:developfrom
myzhang-1127:develop
Open

Convert Accumulators documentation from QuickBook to AsciiDoc/Antora#1
myzhang-1127 wants to merge 6 commits intoCppDigest:developfrom
myzhang-1127:develop

Conversation

@myzhang-1127
Copy link
Copy Markdown
Collaborator

@myzhang-1127 myzhang-1127 commented Apr 19, 2026

Convert legacy QuickBook (qbk) documentation to modern AsciiDoc (adoc) format for better readability, maintainability, and tooling compatibility. No code changes.

Summary by CodeRabbit

Release Notes

  • Documentation
    • Migrated documentation to a modern Antora-based platform with improved navigation and structure.
    • Added keyboard navigation shortcuts for scrolling through documentation pages.
    • Enhanced documentation styling with better code block formatting and responsive layout.
    • Integrated search functionality and tab support for improved documentation browsing experience.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 19, 2026

📝 Walkthrough

Walkthrough

This PR introduces a complete Antora-based documentation build system for Boost.Accumulators, adding build configuration, playbook and site settings, new documentation pages and navigation structure, and supplemental UI components including custom styling and JavaScript for keyboard navigation and scroll behavior management.

Changes

Cohort / File(s) Summary
Build & Tooling Configuration
doc/Jamfile.v2, doc/build_antora.sh, doc/build_antora.sh.bak, doc/package.json
Adds Antora build targets to Jamfile, introduces build scripts that auto-detect build directories, run npm dependencies, invoke Antora, and post-process generated site assets; defines Node dependencies for Antora CLI, site generator, and extensions.
Antora Configuration
doc/antora.yml, doc/local-playbook.yml
Defines Antora component metadata (accumulators with floating version, start page, AsciiDoc attributes) and playbook configuration (site title, UI bundle, content source, extensions for Lunr search and Boost link handling).
Documentation Content
doc/modules/ROOT/nav.adoc, doc/modules/ROOT/pages/index.adoc, doc/modules/ROOT/pages/preface.adoc, doc/modules/ROOT/pages/acknowledgements.adoc
Establishes documentation structure via navigation file and adds four AsciiDoc pages covering landing page, preface, acknowledgements, and cross-references to guide, reference, and framework sections.
UI Styling
doc/supplemental-ui/css/accumulators-dlist.css, doc/supplemental-ui/css/accumulators-legacy-html.css, doc/supplemental-ui/css/boostlook-site.css
Adds theme-aware CSS for mini-TOC, description lists, legacy BoostBook HTML navigation layout, and scroll-margin adjustments for Antora-rendered documentation.
UI Scripts & Templates
doc/supplemental-ui/partials/footer-scripts.hbs, doc/supplemental-ui/js/accumulators-keyboard-nav.js
Introduces Handlebars footer template that injects scripts for syntax highlighting, tab sync, Lunr search, scroll position restoration, and hash-based navigation; adds JavaScript module for PageUp/PageDown/Home keyboard scrolling in documentation content areas.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A burrow of docs, now built with such care,
Antora weaves through the Boost library fair,
With keyboard hops and smooth scrolling grace,
We've hopped-up this documentation space! ✨📚

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and specifically summarizes the main change: converting documentation from QuickBook to AsciiDoc/Antora format, which is the primary focus of all modified and added files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/build_antora.sh`:
- Around line 75-129: The build currently comments out the legacy Doxygen
reference installation and patch steps (the REF_DST/IMG_DST/LEGREF blocks and
the calls to tools/fix_framework_ref_html.py and
tools/fix_reference_html_paths.py), so Antora output may be missing copied
Doxygen HTML/assets; either uncomment and restore the REF_DST/IMG_DST/LEGREF
sections (re-enabling the cp/rm and the python fix_* scripts that reference
SCRIPT_DIR and BOOST_SRC_DIR) so accumulated docs are copied to _/ref and
patched, or if you no longer want the Doxygen path, remove related build steps
and update docs that reference /_/ref/... and any references to BOOST_SRC_DIR,
fix_framework_ref_html.py and fix_reference_html_paths.py to avoid dangling
links.

In `@doc/Jamfile.v2`:
- Around line 265-271: The antora-html target invokes bash via the action named
run-accumulators-antora (bash $(>)) which will fail on Windows; either guard the
target on NT by checking [ os.name ] = NT in the Jamfile and make make
antora-html emit a clear error or no-op on NT, or change the
run-accumulators-antora action to invoke a resolved POSIX shell (e.g., use sh if
available or a configured interpreter variable) and document the requirement;
update the action definition (run-accumulators-antora) and the explicit
antora-html target to implement the chosen guard or interpreter change and add a
visible comment/error message for NT users.

In `@doc/local-playbook.yml`:
- Line 7: The placeholder site.url entry currently set to
"https://example.org/boost-accumulators" will be baked into generated
canonicals/sitemap/Lunr links; either replace the site.url value with your real
production deployment URL (e.g., the actual Boost docs URL) or remove the
site.url (and related robots) keys from this playbook if it is only for local
preview and rename the file to indicate local/dev use so the placeholder doesn't
leak into production output.

In `@doc/modules/ROOT/pages/acknowledgements.adoc`:
- Around line 2-4: Remove the duplicate AsciiDoc anchor by keeping only one of
the two declarations: either delete the attribute line ":id:
accumulators.acknowledgements" or remove the explicit anchor line
"[[accumulators.acknowledgements]]" so only a single anchor remains; update the
file to leave just one of these identifiers to avoid duplicate-id warnings.

In `@doc/modules/ROOT/pages/preface.adoc`:
- Around line 2-4: The page currently declares the same anchor twice (:id:
accumulators.preface and [[accumulators.preface]]); remove one of them to avoid
duplicate-anchor warnings—prefer keeping the top-level attribute ':id:
accumulators.preface' and delete the duplicate inline anchor
'[[accumulators.preface]]' (or vice versa) so the page has a single id
declaration.

In `@doc/supplemental-ui/css/accumulators-dlist.css`:
- Around line 25-48: The .doc .dlist > dl block currently hardcodes light colors
(background, color, link colors) which breaks dark mode; add corresponding
html.dark overrides (similar to the existing .spirit-nav-footer-wrap override)
for the selectors .doc .dlist > dl, .doc .dlist > dl a, .doc .dlist > dl code
and the .accumulators-mini-toc panel so those elements get dark-theme colors
when html.dark is present; keep the existing light defaults but add the
dark-mode rule-set to flip background, text, and link colors appropriately to
match the Antora night theme.

In `@doc/supplemental-ui/partials/footer-scripts.hbs`:
- Around line 4-8: Change the three script tags that load lunr.js, search-ui.js
(id="search-ui-script"), and search-index.js to use the defer attribute instead
of async so they are fetched in parallel but executed in document order; ensure
the order remains lunr.js first, search-ui.js (which relies on lunr and uses the
{{{uiRootPath}}} and id="search-ui-script"), then search-index.js (using
{{{siteRootPath}}}) to preserve dependencies and avoid intermittent
initialization failures.

In `@doc/tools/import_reference_from_official.py`:
- Around line 102-143: convert_reference_ordered currently builds flow =
inner.find_all(["h3","h4","h5","h6","pre"]) which drops paragraphs, lists,
tables and other prose; update the collector and processing so non-heading
content is preserved: expand flow to include block-level tags (e.g.,
"p","ul","ol","table","div" with class="sect" etc.) or iterate over
inner.children and handle nodes by type, adding handlers similar to the pre path
(implement emitters like _emit_paragraph, _emit_list, _emit_table or
reuse/_extend _emit_pre logic) and ensure these emitters are called only after
started is True; keep existing header handling in convert_reference_ordered and
use _escape_adoc/_section_depth_from_tag to format names/titles consistently.
- Around line 42-73: The inline conversion comprehensions in _convert_inline use
the wrong loop variable (they call _convert_inline(child) while iterating "for c
in child.children"), causing same-node recursion; update each comprehension in
the link handling block and the emphasis/strong blocks to call
_convert_inline(c) when isinstance(c, Tag) (i.e., replace _convert_inline(child)
with _convert_inline(c)) so nested tags are processed correctly with the correct
element.
- Line 152: The code directly calls urllib.request.urlopen(args.url) which can
open non-HTTP(S) schemes (like file://); update the logic before the fetch in
import_reference_from_official.py to parse and validate args.url (e.g., using
urllib.parse.urlparse) and only allow scheme 'http' or 'https', raising an error
or exiting if the scheme is anything else, then proceed to call
urllib.request.urlopen(args.url, timeout=120) to populate html_text; reference
the existing html_text assignment, args.url, and urllib.request.urlopen when
making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 71b73a6f-3ad5-47c4-9e65-f73fb20ea80b

📥 Commits

Reviewing files that changed from the base of the PR and between 6bd97b3 and 318172a.

⛔ Files ignored due to path filters (1)
  • doc/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • doc/.gitignore
  • doc/DOCUMENTATION_WORK_PRINCIPLES.md
  • doc/Jamfile.v2
  • doc/OFFICIAL_PARITY.md
  • doc/README.md
  • doc/accdoc.xml
  • doc/antora.yml
  • doc/build_antora.sh
  • doc/local-playbook.yml
  • doc/modules/ROOT/nav.adoc
  • doc/modules/ROOT/pages/acknowledgements.adoc
  • doc/modules/ROOT/pages/index.adoc
  • doc/modules/ROOT/pages/preface.adoc
  • doc/modules/ROOT/pages/reference.adoc
  • doc/modules/ROOT/pages/user_s_guide.adoc
  • doc/opdoc.xml
  • doc/package.json
  • doc/statsdoc.xml
  • doc/supplemental-ui/css/accumulators-dlist.css
  • doc/supplemental-ui/css/accumulators-legacy-html.css
  • doc/supplemental-ui/css/boostlook-site.css
  • doc/supplemental-ui/partials/footer-scripts.hbs
  • doc/tagfile.xml
  • doc/tools/fix_framework_ref_html.py
  • doc/tools/fix_reference_html_paths.py
  • doc/tools/import_reference_from_official.py

Comment thread doc/build_antora.sh Outdated
Comment thread doc/Jamfile.v2
Comment thread doc/local-playbook.yml

site:
title: Boost.Accumulators
url: https://example.org/boost-accumulators
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Placeholder site.url will leak into generated site.

https://example.org/boost-accumulators is not the intended production URL. Antora uses site.url to generate canonical links, sitemap entries, and absolute URLs (e.g., for the Lunr index). Shipping this value will produce a sitemap/robots/canonicals pointing at example.org.

Either set the real deployment URL (e.g., https://www.boost.org/doc/libs/...) or, if this playbook is only meant for local previewing, drop site.url / robots and rename the file's intent accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/local-playbook.yml` at line 7, The placeholder site.url entry currently
set to "https://example.org/boost-accumulators" will be baked into generated
canonicals/sitemap/Lunr links; either replace the site.url value with your real
production deployment URL (e.g., the actual Boost docs URL) or remove the
site.url (and related robots) keys from this playbook if it is only for local
preview and rename the file to indicate local/dev use so the placeholder doesn't
leak into production output.

Comment thread doc/modules/ROOT/pages/acknowledgements.adoc Outdated
Comment thread doc/modules/ROOT/pages/preface.adoc Outdated
Comment thread doc/supplemental-ui/css/accumulators-dlist.css
Comment thread doc/supplemental-ui/partials/footer-scripts.hbs Outdated
Comment thread doc/tools/import_reference_from_official.py Outdated
Comment thread doc/tools/import_reference_from_official.py Outdated
Comment thread doc/tools/import_reference_from_official.py Outdated
Copy link
Copy Markdown

@wpak-ai wpak-ai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do the following:

  1. Address all coderabbit comments (Critical and Major).
  2. Address my comments.
  3. Refer to boost.url's doc directory for what files are needed for adoc-style documentation: https://github.com/boostorg/url/tree/develop/doc

Comment thread doc/README.md Outdated
@@ -0,0 +1,75 @@
# Boost.Accumulators documentation
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boost.url doesn't have a doc/README.md.
This file should be removed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment thread doc/OFFICIAL_PARITY.md Outdated
@@ -0,0 +1,66 @@
# Parity vs official Boost.Accumulators HTML (boost.org)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment thread doc/tagfile.xml Outdated
@@ -0,0 +1,54 @@
<?xml version="1.0" standalone="yes"?>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment thread doc/DOCUMENTATION_WORK_PRINCIPLES.md Outdated
@@ -0,0 +1,51 @@
# 文档与转换工作原则
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely remove!!!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@@ -0,0 +1,100 @@
/* accumulators-dlist-injected — appended once by build_antora.sh; do not edit site.css by hand */
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need extra css files?
isn't boostlook enough?
this is error-prone if boostlook evolves later.

Comment thread doc/tools/fix_framework_ref_html.py Outdated
@@ -0,0 +1,56 @@
#!/usr/bin/env python3
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose these are extra files that were necessary when converting qbk to adoc.
Do we need these files whenever boost rebuilds all documentation?
The rule should if boost rebuilds adoc files for boost.accumulators, do we need these files?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment thread doc/accdoc.xml Outdated
@@ -0,0 +1,54 @@
<?xml version="1.0" standalone="yes"?>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an output when converting qbk, so it should be removed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment thread doc/local-playbook.yml
@@ -0,0 +1,36 @@
#
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this file?

Copy link
Copy Markdown
Collaborator Author

@myzhang-1127 myzhang-1127 Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is necessary when we are building html by antora with adocs and this file also exists on the url/doc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this file.

Comment thread doc/opdoc.xml Outdated
@@ -0,0 +1,2 @@
<?xml version="1.0" standalone="yes"?>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
doc/build_antora.sh.bak (2)

75-129: Remove or externalize large commented execution blocks.

This block is effectively dormant code and makes the script harder to maintain. Prefer removing it (and documenting restoration steps in a markdown/adoc note) or gating real logic behind explicit flags.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/build_antora.sh.bak` around lines 75 - 129, The large dormant commented
blocks (refs using REF_DST, IMG_DST, BOOST_SRC_DIR, LEGREF and calls to
tools/fix_framework_ref_html.py and tools/fix_reference_html_paths.py) should be
removed from doc/build_antora.sh.bak or moved into an external restoreable
script (e.g., legacy_assets.sh) and referenced from the main script;
alternatively gate the logic behind an explicit env flag (e.g.,
USE_LEGACY_ASSETS) so the commands (mkdir -p, cp, rm -rf and Python invocations)
run only when the flag is set, and add a short markdown/adoc note explaining how
to restore or enable the legacy behavior and where the extracted script lives.

1-131: Avoid keeping executable .bak build scripts in-tree.

Keeping doc/build_antora.sh.bak alongside the active script invites drift and accidental usage. Prefer deleting it from source control or converting it to non-executable archival docs if needed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/build_antora.sh.bak` around lines 1 - 131, The PR contains an executable
backup script named build_antora.sh.bak (starts with #!/usr/bin/env bash and
defines SCRIPT_DIR/PLAYBOOK), which should not be kept in-tree as an executable;
remove the .bak file from the repository (git rm) or move it to a non-executable
archival location (e.g., docs/archive) and ensure its executable bit is cleared,
and if needed add an entry to .gitignore or a dedicated archive dir so
accidental usage is prevented and no build tooling references the .bak name
(verify no references to build_antora.sh.bak remain).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/build_antora.sh.bak`:
- Around line 34-38: Current argument handling silently ignores extra CLI args:
change the conditional to explicitly validate arity by handling three cases: if
[ "$#" -eq 0 ] then set PLAYBOOK="local-playbook.yml"; elif [ "$#" -eq 1 ] then
set PLAYBOOK="$1"; else print a short usage message to stderr (e.g., "Usage:
script.sh [playbook.yml]") and exit 1; update references to PLAYBOOK and keep
the existing behavior when exactly one or zero args are provided.

In `@doc/supplemental-ui/partials/footer-scripts.hbs`:
- Around line 149-152: The cloned navigation landmark created by wrap (class
'spirit-nav-footer-wrap') uses role="navigation" but has no accessible name; add
an accessible name to that landmark by setting an aria-label or aria-labelledby
on the wrap (or on the cloned node referenced by bottom =
toolbarNav.cloneNode(true)) such as "Footer navigation" (or pointing to a
visible heading ID) so screen readers can distinguish this footer nav from other
navigation regions.
- Around line 29-49: The back/forward scroll save/restore currently only uses
window.scrollY and window.scrollTo, which fails when pages use an inner scroller
(the article container like `#content`) as implemented by
accumulators-keyboard-nav.js; update the handlers that call scrollKey(), parse
and restore to detect whether the article scroller exists and then save/restore
that element's scrollTop instead of (or in addition to) window scroll: in the
pagehide listener (where sessionStorage.setItem(scrollKey(), ... ) is called)
check document.querySelector('#content') (or the same selector used by
accumulators-keyboard-nav.js) and store that element’s scrollTop when present;
in the pageshow logic (around isBackForwardNavigation(), applyScroll, and
requestAnimationFrame) detect the same scroller and call element.scrollTop = n
(or element.scrollTo) to restore, falling back to window.scrollTo(0, n) when no
scroller is found; keep using scrollKey() and isBackForwardNavigation() to
locate keys and navigation checks.

---

Nitpick comments:
In `@doc/build_antora.sh.bak`:
- Around line 75-129: The large dormant commented blocks (refs using REF_DST,
IMG_DST, BOOST_SRC_DIR, LEGREF and calls to tools/fix_framework_ref_html.py and
tools/fix_reference_html_paths.py) should be removed from
doc/build_antora.sh.bak or moved into an external restoreable script (e.g.,
legacy_assets.sh) and referenced from the main script; alternatively gate the
logic behind an explicit env flag (e.g., USE_LEGACY_ASSETS) so the commands
(mkdir -p, cp, rm -rf and Python invocations) run only when the flag is set, and
add a short markdown/adoc note explaining how to restore or enable the legacy
behavior and where the extracted script lives.
- Around line 1-131: The PR contains an executable backup script named
build_antora.sh.bak (starts with #!/usr/bin/env bash and defines
SCRIPT_DIR/PLAYBOOK), which should not be kept in-tree as an executable; remove
the .bak file from the repository (git rm) or move it to a non-executable
archival location (e.g., docs/archive) and ensure its executable bit is cleared,
and if needed add an entry to .gitignore or a dedicated archive dir so
accidental usage is prevented and no build tooling references the .bak name
(verify no references to build_antora.sh.bak remain).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 13251764-2da8-45a9-aaca-56681cebd432

📥 Commits

Reviewing files that changed from the base of the PR and between 318172a and c09b49b.

📒 Files selected for processing (8)
  • doc/accumulators.qbk
  • doc/build_antora.sh
  • doc/build_antora.sh.bak
  • doc/modules/ROOT/pages/acknowledgements.adoc
  • doc/modules/ROOT/pages/preface.adoc
  • doc/supplemental-ui/css/accumulators-dlist.css
  • doc/supplemental-ui/js/accumulators-keyboard-nav.js
  • doc/supplemental-ui/partials/footer-scripts.hbs
✅ Files skipped from review due to trivial changes (3)
  • doc/modules/ROOT/pages/acknowledgements.adoc
  • doc/build_antora.sh
  • doc/supplemental-ui/css/accumulators-dlist.css
🚧 Files skipped from review as they are similar to previous changes (1)
  • doc/modules/ROOT/pages/preface.adoc

Comment thread doc/build_antora.sh.bak
Comment on lines +34 to +38
if [ $# -eq 0 ]; then
PLAYBOOK="local-playbook.yml"
else
PLAYBOOK=$1
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate CLI arity to avoid silently ignored arguments.

At Line 34–38, only $1 is consumed and extra args are dropped silently. That can mask invocation mistakes. Please fail fast on "$# > 1" with a short usage message.

Suggested fix
-if [ $# -eq 0 ]; then
+if [ $# -gt 1 ]; then
+  echo "Usage: $0 [playbook.yml]" >&2
+  exit 2
+fi
+
+if [ $# -eq 0 ]; then
   PLAYBOOK="local-playbook.yml"
 else
   PLAYBOOK=$1
 fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ $# -eq 0 ]; then
PLAYBOOK="local-playbook.yml"
else
PLAYBOOK=$1
fi
if [ $# -gt 1 ]; then
echo "Usage: $0 [playbook.yml]" >&2
exit 2
fi
if [ $# -eq 0 ]; then
PLAYBOOK="local-playbook.yml"
else
PLAYBOOK=$1
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/build_antora.sh.bak` around lines 34 - 38, Current argument handling
silently ignores extra CLI args: change the conditional to explicitly validate
arity by handling three cases: if [ "$#" -eq 0 ] then set
PLAYBOOK="local-playbook.yml"; elif [ "$#" -eq 1 ] then set PLAYBOOK="$1"; else
print a short usage message to stderr (e.g., "Usage: script.sh [playbook.yml]")
and exit 1; update references to PLAYBOOK and keep the existing behavior when
exactly one or zero args are provided.

Comment on lines +29 to +49
window.addEventListener('pagehide', function () {
try {
sessionStorage.setItem(scrollKey(), String(window.scrollY || window.pageYOffset || 0));
} catch (e) {}
});
window.addEventListener('pageshow', function (ev) {
if (ev.persisted) return;
try {
if (!isBackForwardNavigation()) return;
var y = sessionStorage.getItem(scrollKey());
if (y == null) return;
var n = parseInt(y, 10);
if (isNaN(n)) return;
function applyScroll() {
window.scrollTo(0, n);
}
setTimeout(applyScroll, 0);
requestAnimationFrame(function () {
requestAnimationFrame(applyScroll);
});
} catch (e) {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Back/forward restoration ignores the actual article scroller.

Line 31 saves only window.scrollY, and Line 43 restores only the window. That misses the layout this PR already supports in doc/supplemental-ui/js/accumulators-keyboard-nav.js, where the article can scroll inside #content. On those pages, back/forward will reopen near the top instead of the reader’s previous position.

Proposed fix
   function scrollKey() {
     return 'accumulatorsDocScroll:' + location.pathname + location.search;
   }
+  function scrollBox() {
+    var el = document.querySelector('.boostlook `#content`') || document.getElementById('content');
+    return el && el.scrollHeight > el.clientHeight ? el : window;
+  }
   function isBackForwardNavigation() {
     var nav = performance.getEntriesByType && performance.getEntriesByType('navigation')[0];
     if (nav && nav.type === 'back_forward') return true;
@@
   window.addEventListener('pagehide', function () {
     try {
-      sessionStorage.setItem(scrollKey(), String(window.scrollY || window.pageYOffset || 0));
+      var box = scrollBox();
+      var y = box === window ? (window.scrollY || window.pageYOffset || 0) : box.scrollTop;
+      sessionStorage.setItem(scrollKey(), String(y));
     } catch (e) {}
   });
@@
       var n = parseInt(y, 10);
       if (isNaN(n)) return;
       function applyScroll() {
-        window.scrollTo(0, n);
+        var box = scrollBox();
+        if (box === window) window.scrollTo(0, n);
+        else box.scrollTop = n;
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/supplemental-ui/partials/footer-scripts.hbs` around lines 29 - 49, The
back/forward scroll save/restore currently only uses window.scrollY and
window.scrollTo, which fails when pages use an inner scroller (the article
container like `#content`) as implemented by accumulators-keyboard-nav.js; update
the handlers that call scrollKey(), parse and restore to detect whether the
article scroller exists and then save/restore that element's scrollTop instead
of (or in addition to) window scroll: in the pagehide listener (where
sessionStorage.setItem(scrollKey(), ... ) is called) check
document.querySelector('#content') (or the same selector used by
accumulators-keyboard-nav.js) and store that element’s scrollTop when present;
in the pageshow logic (around isBackForwardNavigation(), applyScroll, and
requestAnimationFrame) detect the same scroller and call element.scrollTop = n
(or element.scrollTo) to restore, falling back to window.scrollTo(0, n) when no
scroller is found; keep using scrollKey() and isBackForwardNavigation() to
locate keys and navigation checks.

Comment on lines +149 to +152
var wrap = document.createElement('div');
wrap.className = 'spirit-nav-footer-wrap';
wrap.setAttribute('role', 'navigation');
var bottom = toolbarNav.cloneNode(true);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Label the cloned navigation landmark.

Line 151 adds another navigation landmark, but it has no accessible name. Screen readers will expose multiple indistinguishable “navigation” regions on pages that already have header/sidebar nav.

Proposed fix
       var wrap = document.createElement('div');
       wrap.className = 'spirit-nav-footer-wrap';
       wrap.setAttribute('role', 'navigation');
+      wrap.setAttribute('aria-label', 'Page navigation');
       var bottom = toolbarNav.cloneNode(true);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/supplemental-ui/partials/footer-scripts.hbs` around lines 149 - 152, The
cloned navigation landmark created by wrap (class 'spirit-nav-footer-wrap') uses
role="navigation" but has no accessible name; add an accessible name to that
landmark by setting an aria-label or aria-labelledby on the wrap (or on the
cloned node referenced by bottom = toolbarNav.cloneNode(true)) such as "Footer
navigation" (or pointing to a visible heading ID) so screen readers can
distinguish this footer nav from other navigation regions.

@myzhang-1127 myzhang-1127 requested a review from wpak-ai April 21, 2026 19:15
@myzhang-1127 myzhang-1127 changed the title Accumulators:develop(qbk-adoc convert) Convert Accumulators documentation from QuickBook to AsciiDoc/Antora Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants