feat: add selector directive and rocm_docs_pdf_exclude_patterns option - #1615
Conversation
b5818c8 to
4187a7d
Compare
e26b0c2 to
33b84c4
Compare
rocm_docs_pdf_exclude_patterns option
33b84c4 to
c6371cf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 1 comment.
Suppressed comments (14)
src/rocm_docs/selector/static/selector.js:70
- This loop removes every query parameter not present in selector state, including unrelated parameters that the preceding comment promises to preserve (for example,
?highlight=...). Track the selector keys known on the page and delete only stale keys from that set.
for (const key of Array.from(params.keys())) {
if (!(key in state)) {
params.delete(key);
}
src/rocm_docs/selector/utils.py:23
- This says expansion is the default, but the registered default below is
False, matching the user guide's statement that selector content is omitted by default. Correct the shared configuration documentation to avoid misleading extension consumers.
* ``True`` — expand all selector combinations for every page (default).
* ``False`` — omit selector content from the PDF entirely.
src/rocm_docs/selector/init.py:76
- This new public extension has no automated tests, despite comparable extensions using both unit tests and full Sphinx-build fixtures (for example,
tests/test_doxygen.pyandtests/test_llms.py:43-83). Add coverage for directive HTML, URL/visibility behavior, Markdown output, and both boolean and page-scoped LaTeX configurations; the current external manual test links will not protect downstream projects from regressions.
def setup(app):
"""Register all selector nodes, directives, and event hooks with Sphinx."""
src/rocm_docs/selector/static/selector.js:509
- Changing only
opt.disableddoes not update TomSelect's cached option element:refreshOptions()reuses the existing$div, so an option that becomes disabled can retaindata-selectableand still be chosen (and a re-enabled option can remain unselectable). Update the rendered ARIA/data-selectableattributes or invalidate and rebuild the cached option element when this flag changes.
if (opt.disabled !== disabled) {
opt.disabled = disabled;
needsRefresh = true;
}
src/rocm_docs/selector/nodes.py:162
- Registering assets while directives are being read is unsafe with the declared
parallel_read_safe=True: worker processes return their environments, but mutations made byapp.add_js_file/add_css_fileare not merged into the parent app. Parallel builds can therefore render selectors without their JavaScript or CSS. Register these assets from extension setup or an initialization event before parallel reading starts.
_register_selector_assets(self.env)
src/rocm_docs/core.py:169
- The pruning misses cached toctree documents because Sphinx wraps inlined documents in
start_of_file; their tagged sections are descendants rather than directdoctree.children. It also compares source-file glob patterns against extensionless docnames, so exact patterns such asguide/redirect.rstdo not match as Sphinx does. Traverse descendant sections and apply Sphinx's matcher to each section's source path.
for section in list(doctree.children):
if not isinstance(section, nodes.section):
continue
dn = section.get("docname", "")
if dn and any(fnmatch.fnmatch(dn, pat) for pat in patterns):
src/rocm_docs/selector/nodes.py:184
nodes.make_id()can return an empty string for titles beginning with a digit or containing no valid ID characters. Such a selector receivesid="", and repeated selectors produce IDs such as-2. Provide a nonempty fallback before deduplication.
base_id = nodes.make_id(label)
docs/user_guide/selector.rst:314
- The live example defines Oracle Linux as
distro=ol, but this installation-method selector checksdistro=sles. Choosing Oracle Linux therefore hides every installation-method selector, clearsi, and displays none of the documenteddistro=olcontent below.
:show-cond: distro=fedora distro=rhel distro=sles
src/rocm_docs/selector/static/selector.js:35
- Setting every enabled radio tile to
tabindex="0"defeats the roving-tabstop behavior implemented below: keyboard users must Tab through every option in a radiogroup. Keep only the selected option (or one fallback option) at0and set the remaining enabled radios to-1.
const enable = (elem) => {
elem.classList.remove(DISABLED_CLASS);
elem.setAttribute("aria-disabled", "false");
elem.setAttribute("tabindex", "0");
};
src/rocm_docs/selector/nodes.py:115
- This icon-only link has no accessible name, so screen readers announce an unlabeled link. Add an
aria-labelderived from the selector label and mark the decorative icon as hidden; the new-tab link should also userel="noopener noreferrer".
info_icon_html = (
f'<a href="{info_link}" target="_blank">'
f'<i class="rocm-docs-selector-icon {info_icon}"></i>'
f"</a>"
src/rocm_docs/selector/init.py:43
- The user guide says
selector-toc2is required, but an icon-only metadata entry also activates this template and produces an empty sidebar title. Require the title key; the icon should remain optional.
return "selector-toc2" in metadata or "selector-toc2-icon" in metadata
src/rocm_docs/selector/transforms.py:335
- PDF combination discovery ignores each option's
show-condanddisable-cond, retaining only the parent group's condition. As a result, LaTeX expansion generates states that users can never choose in HTML and can emit content for hidden or disabled options. Carry both option conditions into combo generation and filter values against the partial state.
for opt in sg.findall(SelectorOption):
val = opt.get("value", "")
label = opt.get("label", val)
if val and (val, group_show_cond) not in seen[key]:
opts[key].append((val, label, group_show_cond))
seen[key].add((val, group_show_cond))
src/rocm_docs/selector/init.py:54
secondary_sidebar_itemsvalidly accepts a global list in pydata-sphinx-theme, but this branch silently disables selector sidebar injection for that common configuration. Normalize a list to a per-page mapping (preserving its existing defaults) before adding the selector-page override, so the documented metadata works regardless of which supported theme-option form a project uses.
if not isinstance(sidebar, dict):
return
docs/user_guide/selector.rst:456
- This MyST example does not activate the sidebar:
myst.html_metaproduces HTML<meta>nodes, while_has_toc2_metadatareads only Sphinx'senv.metadata, which is populated from the document's top-level field list/docinfo. Put these two custom keys at the top level of the YAML front matter (outsidemyst.html_meta) or update the extension to read meta nodes.
---
myst:
html_meta:
"selector-toc2": "Installation environment"
"selector-toc2-icon": "fa-solid fa-computer"
38ad60c to
e11cca5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (6)
src/rocm_docs/selector/static/selector-toc2.js:81
- The visible label is not associated with this select:
htmlForis set to the heading text, but the select has no matchingid, and TomSelect therefore cannot discover the label when it initializes. Assign a unique ID to the select and pointlabel.htmlForto it so clicking the label focuses the control and assistive technology preserves the explicit association.
const label = document.createElement("label");
label.className = DROPDOWN_INPUT_LABEL_CLASS;
label.textContent = headingText;
label.htmlFor = headingText;
const selectEl = document.createElement("select");
selectEl.className = `form-select ${DROPDOWN_INPUT_CLASS}`;
selectEl.name = headingText;
selectEl.setAttribute("aria-label", headingText);
src/rocm_docs/selector/transforms.py:600
- Only direct
SelectedContentchildren are detected here. If conditional content is wrapped in a supported container such as an admonition or dropdown,all_selectedis nonempty butfirst_selected_idxremainsNone, so PDF reorganization is skipped and a page-scoped mock state later unwraps every variant instead of filtering it. Locate the top-level child that contains the first selected node and pass that container through_gather_content.
for i, child in enumerate(chapter.children):
if isinstance(child, SelectedContent):
first_selected_idx = i
break
src/rocm_docs/selector/transforms.py:439
- Version sub-selectors bypass the visibility checks applied to primary selectors: every
ver_entriesitem is expanded even when its group or optionshow-condis false for the current combo. This produces PDF sections for choices that the HTML selector cannot expose. Filter version entries with_visible_in_partial_combo, and recurse without the version key when the sub-selector has no visible values.
if ver_entries:
for ver_val, ver_label, _, _opt_cond in ver_entries:
combo[ver_key] = ver_val
labels[ver_key] = ver_label
_iter_combos(rest, sel_opts, combo, labels, out)
src/rocm_docs/selector/nodes.py:39
- This checks descendants of
state.parent, not whether the directive's parent is a selector group. Once a document already contains any selector, a later top-levelselector-optionorselector-infoincorrectly passes this check and renders with no group key. Check the actual parent relationship instead.
parent = getattr(state, "parent", None)
if not parent or not any(
isinstance(p, SelectorGroup) for p in parent.traverse(include_self=True)
):
src/rocm_docs/selector/transforms.py:337
- The option's
disable-condis never collected, so the PDF combo builder expands options that are disabled in the interactive selector. This contradicts_iter_combos's valid-choice behavior and can emit impossible selector states. Carry the disable condition through the option tuple and exclude an option once that condition definitively matches the combo.
group_show_cond = sg.get("show-cond", "")
for opt in sg.findall(SelectorOption):
val = opt.get("value", "")
label = opt.get("label", val)
option_show_cond = opt.get("show-cond", "")
if val and (val, group_show_cond) not in seen[key]:
opts[key].append((val, label, group_show_cond, option_show_cond))
docs/sphinx/_toc.yml.in:14
- Removing both guide landing pages from the TOC while emptying their source files leaves the published URLs linked from
README.md:39andREADME.md:45as blank/orphan pages. Preserve these landing pages (or add redirects) and update the repository links so existing entry points continue to lead to useful content.
- caption: User guide
entries:
8bdadcc to
19e650d
Compare
|
Checked the generated pdf file. LGTM. I only have one comment. |
8aacd68 to
4dc7393
Compare
virtualenv==21.5.1 requires python-discovery>=1.4.2 but this transitive dependency was missing from requirements.txt, causing pre-commit to fail with 'ModuleNotFoundError: No module named python_discovery' when trying to create a virtualenv environment." Co-authored-by: peterjunpark <115042610+peterjunpark@users.noreply.github.com>
a6f499f to
1c3f641
Compare
0179bd5 to
ab31478
Compare
8805e6c to
c9da01d
Compare
Co-authored-by: peterjunpark <115042610+peterjunpark@users.noreply.github.com> style(selector/nodes.py): sort imports to fix isort lint run black fmt fmt
c9da01d to
68ed0fa
Compare
Motivation
Add rocm_docs.selector extension
Adds an optional Sphinx extension that lets documentation authors embed interactive selector widgets on any page. Readers click tiles or choose from dropdowns to filter page content by OS, GPU, install method, or any other dimension —
only the content matching the active selection is shown.
What's new
New extension: rocm_docs.selector
Docs
Technical Details
Ports the selector Sphinx extension from the rocm repo into rocm-docs-core as rocm_docs.selector, making it available to any project that depends on rocm-docs-core.
New package: src/rocm_docs/selector/
pyproject.toml — updated to package static assets under selector/
Test Plan
Test Result
LGTM
Submission Checklist