feat: migrate from Sphinx to MkDocs Material#177
Merged
Conversation
- Replace Sphinx (sphinx_immaterial) with MkDocs (Material for MkDocs) to unify the documentation engine across all cpp-linter projects - Add mkdocs.yml with indigo color scheme matching the hub site - Convert README.rst to README.md (Issue #8) - Convert all docs pages from RST to Markdown - Add gen-files plugin for auto-generating CLI reference pages - Rename new_favicon.ico to favicon.ico (Issue #7) - Add '← cpp-linter hub' back-navigation link (Issue #10) - Add MkDocs deploy workflow - Update pyproject.toml dependencies from sphinx to mkdocs
Contributor
WalkthroughThis pull request migrates the project documentation from Sphinx/reStructuredText to MkDocs/Markdown. It adds a CLI documentation generator plugin, new MkDocs site configuration, rewrites all docs and the README in Markdown, and updates project dependencies and CI workflows to support the new tooling. ChangesDocumentation System Conversion
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…elete/modify conflict
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
docs/gen_cli_docs.py (2)
7-7: ⚖️ Poor tradeoffAvoid accessing private argparse internals.
The code imports
_StoreTrueAction(line 7) and accessesparser._optionals._actions(line 41), both private implementation details of argparse. This coupling is fragile and may break across Python versions or argparse updates.Consider using the public API where possible. For example, you can iterate over
parser._actionsdirectly and filter byisinstance(arg, argparse.Action)for store-true detection viaaction == 'store_true'string comparison, though this still requires some introspection.If the current approach is unavoidable for this use case, add a comment explaining the necessity and document the Python/argparse version assumptions.
Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gen_cli_docs.py` at line 7, Replace use of argparse private internals: remove import of _StoreTrueAction and any access to parser._optionals._actions; instead iterate parser._actions (public API) and detect store-true flags by inspecting each Action's .default/.const or comparing its .__class__.__name__ or action string (e.g., action == 'store_true') as appropriate in the function that currently references _StoreTrueAction and parser._optionals._actions; if you absolutely must rely on private attributes, add a clear comment near the code (referencing _StoreTrueAction and parser._optionals._actions) documenting why the private API is required and which Python/argparse versions are assumed.
24-24: 💤 Low valueRemove or document the unused
filenameparameter.The
filenameparameter in_write_cli_docis never used within the function body. Either remove it if unnecessary, or if it's reserved for future use or required by a calling convention, add a docstring note explaining why it's present.🧹 Proposed fix (if truly unused)
-def _write_cli_doc(parser, prog_name: str, filename: str) -> str: +def _write_cli_doc(parser, prog_name: str) -> str: """Generate markdown documentation from an argparse parser."""Then update the call sites on lines 81 and 84:
-_generate("cli_args.md", _write_cli_doc(get_parser(), "clang-tools", "cli_args.md")) +_generate("cli_args.md", _write_cli_doc(get_parser(), "clang-tools")) _generate( "wheel_cli_args.md", - _write_cli_doc(get_wheel_parser(), "clang-tools-wheel", "wheel_cli_args.md"), + _write_cli_doc(get_wheel_parser(), "clang-tools-wheel"), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gen_cli_docs.py` at line 24, The filename parameter on _write_cli_doc is unused; either remove it from the function signature and update all call sites that pass that argument (remove the extra argument) or keep it and add a clear docstring on _write_cli_doc explaining why filename is reserved/unused; ensure the callers that currently supply filename are adjusted to match the new signature if you remove it or leave calls unchanged if you keep it with documentation.Source: Linters/SAST tools
pyproject.toml (1)
54-58: Validate MkDocs minimum dependency versions and compatibility
mkdocs>=1.6: PyPI has1.6.0/1.6.1.mkdocs-material>=9.5: PyPI has9.5.0+and it requiresmkdocs>=1.6,<2(compatible withmkdocs>=1.6).mkdocs-gen-files>=0.5: PyPI has0.5.0and itsrequires_distincludesmkdocs>=1.4.1,<=1.6.1, so the combination of these constraints implicitly capsmkdocsto<=1.6.1—make this explicit if you plan to bumpmkdocsbeyond1.6.1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 54 - 58, The declared docs dependencies in pyproject.toml (mkdocs, mkdocs-material, mkdocs-gen-files) have implicit compatibility constraints: update the mkdocs constraint to reflect mkdocs-gen-files' upper bound by changing "mkdocs>=1.6" to an explicit range (for example "mkdocs>=1.6,<=1.6.1") or relax/remove mkdocs-gen-files if you intend to support mkdocs >1.6.1; ensure the three entries (mkdocs, mkdocs-material, mkdocs-gen-files) are made consistent so mkdocs-material's ">=1.6,<2" and mkdocs-gen-files' ">=1.4.1,<=1.6.1" don't conflict with the declared mkdocs version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/gen_cli_docs.py`:
- Line 9: Remove the unused import Path from the module by deleting the "Path"
import symbol in the import line (the import currently reads "from pathlib
import Path"); ensure no other code references Path in the file (if there are,
replace them with an appropriate alternative), then run the ruff/pre-commit
checks to confirm the unused-import error is resolved.
- Around line 59-65: The current check `if arg.default:` skips valid falsy
defaults (0, False, "", []); change the condition to `if arg.default is not
None:` in gen_cli_docs.py so defaults are rendered even when falsy, keep the
existing conversion logic for list vs scalar (the `isinstance(arg.default,
list)` branch) and continue appending the badge with `badges.append(f"Default:
`{default}`")`.
- Line 46: Replace the runtime assertion "assert arg.help is not None" with
explicit handling: check "if arg.help is None" and either raise a clear
exception (e.g., ValueError(f'Missing help text for argument {arg.name or
arg.dest}')) to signal a contract violation or provide a sensible default/help
placeholder if None is allowed; update the code path that uses "arg.help"
accordingly (look for the variable "arg" and the code that builds CLI docs
around it).
In `@docs/index.md`:
- Line 23: The sentence "Binaries can be specified installed for increased
flexibility" is grammatically unclear; update the line in docs/index.md to a
clear alternative such as "Binaries can be specified or installed for increased
flexibility" or "You can specify binaries to be installed for increased
flexibility"—replace the ambiguous phrase with one of these clearer options and
ensure surrounding punctuation/tense matches the paragraph.
In `@mkdocs.yml`:
- Around line 84-86: The pre-commit check-yaml hook is failing because it
doesn't support PyYAML custom tags like !!python/name: used in pymdownx.emoji in
mkdocs.yml; fix by updating the pre-commit configuration to exclude mkdocs.yml
from the check-yaml hook (add an exclude pattern matching ^mkdocs\.yml$ for the
check-yaml entry) or replace the generic check-yaml hook with a MkDocs-aware
YAML validator so the pymdownx.emoji keys using !!python/name: and
!!python/name:material.extensions.emoji.to_svg are accepted.
In `@README.md`:
- Line 25: Fix the grammatical error in the README sentence "Binaries can be
specified installed for increased flexibility." by updating it to a correct
phrase such as "Binaries can be specified or installed for increased
flexibility." Locate the exact sentence (the fragment "Binaries can be specified
installed") and replace it with the corrected version; ensure the README line
reads clearly as shown.
---
Nitpick comments:
In `@docs/gen_cli_docs.py`:
- Line 7: Replace use of argparse private internals: remove import of
_StoreTrueAction and any access to parser._optionals._actions; instead iterate
parser._actions (public API) and detect store-true flags by inspecting each
Action's .default/.const or comparing its .__class__.__name__ or action string
(e.g., action == 'store_true') as appropriate in the function that currently
references _StoreTrueAction and parser._optionals._actions; if you absolutely
must rely on private attributes, add a clear comment near the code (referencing
_StoreTrueAction and parser._optionals._actions) documenting why the private API
is required and which Python/argparse versions are assumed.
- Line 24: The filename parameter on _write_cli_doc is unused; either remove it
from the function signature and update all call sites that pass that argument
(remove the extra argument) or keep it and add a clear docstring on
_write_cli_doc explaining why filename is reserved/unused; ensure the callers
that currently supply filename are adjusted to match the new signature if you
remove it or leave calls unchanged if you keep it with documentation.
In `@pyproject.toml`:
- Around line 54-58: The declared docs dependencies in pyproject.toml (mkdocs,
mkdocs-material, mkdocs-gen-files) have implicit compatibility constraints:
update the mkdocs constraint to reflect mkdocs-gen-files' upper bound by
changing "mkdocs>=1.6" to an explicit range (for example "mkdocs>=1.6,<=1.6.1")
or relax/remove mkdocs-gen-files if you intend to support mkdocs >1.6.1; ensure
the three entries (mkdocs, mkdocs-material, mkdocs-gen-files) are made
consistent so mkdocs-material's ">=1.6,<2" and mkdocs-gen-files'
">=1.4.1,<=1.6.1" don't conflict with the declared mkdocs version.
🪄 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: 118b38c0-dd45-4a2c-93a5-b737339207b6
⛔ Files ignored due to path filters (2)
docs/images/favicon.icois excluded by!**/*.icodocs/images/logo.pngis excluded by!**/*.png
📒 Files selected for processing (14)
README.mdREADME.rstdocs/api.mddocs/api.rstdocs/conf.pydocs/gen_cli_docs.pydocs/index.mddocs/index.rstdocs/stylesheets/extra.cssdocs/usage.mddocs/usage.rstdocs/wheel_cli_args.rstmkdocs.ymlpyproject.toml
💤 Files with no reviewable changes (6)
- docs/index.rst
- docs/api.rst
- README.rst
- docs/usage.rst
- docs/conf.py
- docs/wheel_cli_args.rst
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Line 188: The workflow currently references the reusable workflow with a
mutable ref "uses: cpp-linter/.github/.github/workflows/mkdocs.yml@main";
replace that mutable ref with an immutable full commit SHA for the target
repository (e.g., change the "`@main`" suffix to the specific commit SHA) so the
reusable workflow invocation is pinned; update the line that contains the "uses:
cpp-linter/.github/.github/workflows/mkdocs.yml@main" reference to use the
chosen full commit SHA.
In @.pre-commit-config.yaml:
- Line 12: Remove the exclusion that skips YAML validation for mkdocs.yml in the
pre-commit configuration by deleting or modifying the exclude pattern `exclude:
^mkdocs\.yml$` in the .pre-commit-config.yaml so that the YAML hook runs against
mkdocs.yml; ensure the YAML hook entry (the YAML linter / hook configuration) no
longer excludes `mkdocs.yml`.
🪄 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: 250d0687-7f01-4014-b0c3-0507948e93c5
📒 Files selected for processing (4)
.github/workflows/test.yml.pre-commit-config.yamldocs/gen_cli_docs.pypyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/gen_cli_docs.py
| uses: cpp-linter/.github/.github/workflows/sphinx.yml@main | ||
| with: | ||
| path-to-doc: docs/_build/html | ||
| uses: cpp-linter/.github/.github/workflows/mkdocs.yml@main |
Contributor
There was a problem hiding this comment.
Pin the reusable workflow to an immutable commit SHA.
Line 188 references @main, which is mutable and can change behavior unexpectedly or be abused via supply-chain compromise. Pin to a full commit SHA.
Suggested fix
- docs:
- uses: cpp-linter/.github/.github/workflows/mkdocs.yml@main
+ docs:
+ uses: cpp-linter/.github/.github/workflows/mkdocs.yml@<full_commit_sha>🧰 Tools
🪛 zizmor (1.25.2)
[error] 188-188: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml at line 188, The workflow currently references
the reusable workflow with a mutable ref "uses:
cpp-linter/.github/.github/workflows/mkdocs.yml@main"; replace that mutable ref
with an immutable full commit SHA for the target repository (e.g., change the
"`@main`" suffix to the specific commit SHA) so the reusable workflow invocation
is pinned; update the line that contains the "uses:
cpp-linter/.github/.github/workflows/mkdocs.yml@main" reference to use the
chosen full commit SHA.
Source: Linters/SAST tools
| - id: check-docstring-first | ||
| - id: check-added-large-files | ||
| - id: check-yaml | ||
| exclude: ^mkdocs\.yml$ |
Contributor
There was a problem hiding this comment.
Keep mkdocs.yml under YAML validation.
Line 12 disables YAML checks for mkdocs.yml, which can allow invalid config to merge and break docs build/deploy flows later.
Suggested fix
- id: check-yaml
- exclude: ^mkdocs\.yml$📝 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
| exclude: ^mkdocs\.yml$ | |
| - id: check-yaml |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.pre-commit-config.yaml at line 12, Remove the exclusion that skips YAML
validation for mkdocs.yml in the pre-commit configuration by deleting or
modifying the exclude pattern `exclude: ^mkdocs\.yml$` in the
.pre-commit-config.yaml so that the YAML hook runs against mkdocs.yml; ensure
the YAML hook entry (the YAML linter / hook configuration) no longer excludes
`mkdocs.yml`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
=======================================
Coverage 99.64% 99.64%
=======================================
Files 9 9
Lines 558 558
=======================================
Hits 556 556
Misses 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…D043/MD046 - Replace assert with ValueError in docs/gen_cli_docs.py (Bandit B101) - Add .markdownlint.yaml to configure MD013 (line-length: 160), MD043 (disabled for MkDocs pages), MD046 (fenced code blocks)
- Remove .markdownlint.yaml (revert - not needed for this project) - Replace private argparse APIs (_StoreTrueAction, _optionals._actions) with public equivalents - Remove unused filename parameter from _write_cli_doc() - Fix falsy default check: 'if arg.default:' -> 'if arg.default is not None:' - Fix grammar: 'specified installed' -> 'specified or installed'
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/gen_cli_docs.py (1)
41-42: Replace hard-coded"==SUPPRESS=="withargparse.SUPPRESS
Use the exportedargparse.SUPPRESSsentinel for this check (stdlib guidance; the literal value is an internal implementation detail). Update the condition accordingly (and addimport argparseif it isn’t already present).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gen_cli_docs.py` around lines 41 - 42, The code checks for the argparse suppress sentinel using the hard-coded string "==SUPPRESS=="; update the condition in the block that references aliases and arg.default to compare against argparse.SUPPRESS instead (i.e., use argparse.SUPPRESS in the if statement), and add an import argparse at top if it's not already imported; target the variables/expressions aliases and arg.default in this change so the check uses the exported sentinel.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/gen_cli_docs.py`:
- Around line 41-42: The code checks for the argparse suppress sentinel using
the hard-coded string "==SUPPRESS=="; update the condition in the block that
references aliases and arg.default to compare against argparse.SUPPRESS instead
(i.e., use argparse.SUPPRESS in the if statement), and add an import argparse at
top if it's not already imported; target the variables/expressions aliases and
arg.default in this change so the check uses the exported sentinel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96a3676d-7003-4606-a089-412fbf108bab
📒 Files selected for processing (6)
README.mddocs/api.mddocs/gen_cli_docs.pydocs/index.mddocs/usage.mdmkdocs.yml
✅ Files skipped from review due to trivial changes (2)
- README.md
- mkdocs.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/api.md
- docs/usage.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate documentation from Sphinx (
sphinx_immaterial) to MkDocs (Material for MkDocs) to unify the docs engine across all cpp-linter projects.Fixes issues: #6 (docs engine), #7 (favicon naming), #8 (README format), #10 (back navigation)
Changes
Documentation engine (#6)
mkdocs.ymlwithindigocolor scheme matching the hub sitecpp-linter/.githubaction)Favicon naming (#7)
docs/_static/new_favicon.ico→docs/images/favicon.icoREADME format (#8)
README.rst→README.md(now consistent with all other repos)pyproject.tomlreadmefield and docs dependenciesBack navigation (#10)
https://cpp-linter.github.io/Content
Summary by CodeRabbit
Documentation
Chores