Convert Accumulators documentation from QuickBook to AsciiDoc/Antora#1
Convert Accumulators documentation from QuickBook to AsciiDoc/Antora#1myzhang-1127 wants to merge 6 commits intoCppDigest:developfrom
Conversation
Made-with: Cursor
📝 WalkthroughWalkthroughThis 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (1)
doc/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
doc/.gitignoredoc/DOCUMENTATION_WORK_PRINCIPLES.mddoc/Jamfile.v2doc/OFFICIAL_PARITY.mddoc/README.mddoc/accdoc.xmldoc/antora.ymldoc/build_antora.shdoc/local-playbook.ymldoc/modules/ROOT/nav.adocdoc/modules/ROOT/pages/acknowledgements.adocdoc/modules/ROOT/pages/index.adocdoc/modules/ROOT/pages/preface.adocdoc/modules/ROOT/pages/reference.adocdoc/modules/ROOT/pages/user_s_guide.adocdoc/opdoc.xmldoc/package.jsondoc/statsdoc.xmldoc/supplemental-ui/css/accumulators-dlist.cssdoc/supplemental-ui/css/accumulators-legacy-html.cssdoc/supplemental-ui/css/boostlook-site.cssdoc/supplemental-ui/partials/footer-scripts.hbsdoc/tagfile.xmldoc/tools/fix_framework_ref_html.pydoc/tools/fix_reference_html_paths.pydoc/tools/import_reference_from_official.py
|
|
||
| site: | ||
| title: Boost.Accumulators | ||
| url: https://example.org/boost-accumulators |
There was a problem hiding this comment.
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.
wpak-ai
left a comment
There was a problem hiding this comment.
Let's do the following:
- Address all coderabbit comments (Critical and Major).
- Address my comments.
- Refer to boost.url's doc directory for what files are needed for adoc-style documentation: https://github.com/boostorg/url/tree/develop/doc
| @@ -0,0 +1,75 @@ | |||
| # Boost.Accumulators documentation | |||
There was a problem hiding this comment.
boost.url doesn't have a doc/README.md.
This file should be removed.
| @@ -0,0 +1,66 @@ | |||
| # Parity vs official Boost.Accumulators HTML (boost.org) | |||
| @@ -0,0 +1,54 @@ | |||
| <?xml version="1.0" standalone="yes"?> | |||
| @@ -0,0 +1,51 @@ | |||
| # 文档与转换工作原则 | |||
| @@ -0,0 +1,100 @@ | |||
| /* accumulators-dlist-injected — appended once by build_antora.sh; do not edit site.css by hand */ | |||
There was a problem hiding this comment.
why do we need extra css files?
isn't boostlook enough?
this is error-prone if boostlook evolves later.
| @@ -0,0 +1,56 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
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?
| @@ -0,0 +1,54 @@ | |||
| <?xml version="1.0" standalone="yes"?> | |||
There was a problem hiding this comment.
I think this is an output when converting qbk, so it should be removed.
| @@ -0,0 +1,36 @@ | |||
| # | |||
There was a problem hiding this comment.
This file is necessary when we are building html by antora with adocs and this file also exists on the url/doc.
There was a problem hiding this comment.
We need this file.
| @@ -0,0 +1,2 @@ | |||
| <?xml version="1.0" standalone="yes"?> | |||
There was a problem hiding this comment.
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.bakbuild scripts in-tree.Keeping
doc/build_antora.sh.bakalongside 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
📒 Files selected for processing (8)
doc/accumulators.qbkdoc/build_antora.shdoc/build_antora.sh.bakdoc/modules/ROOT/pages/acknowledgements.adocdoc/modules/ROOT/pages/preface.adocdoc/supplemental-ui/css/accumulators-dlist.cssdoc/supplemental-ui/js/accumulators-keyboard-nav.jsdoc/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
| if [ $# -eq 0 ]; then | ||
| PLAYBOOK="local-playbook.yml" | ||
| else | ||
| PLAYBOOK=$1 | ||
| fi |
There was a problem hiding this comment.
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.
| 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.
| 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) {} |
There was a problem hiding this comment.
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.
| var wrap = document.createElement('div'); | ||
| wrap.className = 'spirit-nav-footer-wrap'; | ||
| wrap.setAttribute('role', 'navigation'); | ||
| var bottom = toolbarNav.cloneNode(true); |
There was a problem hiding this comment.
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.
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