docs sidebar have long names - #385
Conversation
Issue 151
isuue 151 solved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughSphinx documentation settings were updated, the original object description function is saved, and project-specific pydoclint configuration was added. ChangesDocumentation and docstring tooling
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/source/_static/custom.js`:
- Around line 10-13: The text-shortening logic in custom.js is too broad because
it rewrites any dotted literal, which can incorrectly alter TOC entries like
config.yaml or version strings. Update the handling around the
text.includes(".") branch to only truncate API-style identifiers, using a
stricter guard in the same DOM/TOC processing path so ordinary dotted names and
config keys are left unchanged.
🪄 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: 4353124e-271d-426b-b412-14958409d1f9
📒 Files selected for processing (4)
docs/source/_static/custom.jsdocs/source/conf.pysrc/fdtdx/conversion/vti.pysrc/fdtdx/core/jax/pytrees.py
| // If the text contains a dot (e.g., "ExtrudedPolygon.axis"), keep only the last part ("axis") | ||
| if (text.includes(".")) { | ||
| const parts = text.split("."); | ||
| textTarget.textContent = parts[parts.length - 1]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Narrow the truncation to API identifiers.
This currently rewrites every dotted code literal in the page TOC, so entries like config.yaml, Python 3.12, or dotted config keys can be shortened incorrectly once custom.js is loaded globally.
Proposed guard
- // If the text contains a dot (e.g., "ExtrudedPolygon.axis"), keep only the last part ("axis")
- if (text.includes(".")) {
+ // If the text is a dotted Python identifier (e.g., "ExtrudedPolygon.axis"), keep only the last part ("axis")
+ const isDottedIdentifier = /^[A-Za-z_]\w*(?:\.[A-Za-z_]\w*)+$/.test(text.trim());
+ if (isDottedIdentifier) {
const parts = text.split(".");
+ node.title = text;
textTarget.textContent = parts[parts.length - 1];
}📝 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 the text contains a dot (e.g., "ExtrudedPolygon.axis"), keep only the last part ("axis") | |
| if (text.includes(".")) { | |
| const parts = text.split("."); | |
| textTarget.textContent = parts[parts.length - 1]; | |
| // If the text is a dotted Python identifier (e.g., "ExtrudedPolygon.axis"), keep only the last part ("axis") | |
| const isDottedIdentifier = /^[A-Za-z_]\w*(?:\.[A-Za-z_]\w*)+$/.test(text.trim()); | |
| if (isDottedIdentifier) { | |
| const parts = text.split("."); | |
| node.title = text; | |
| textTarget.textContent = parts[parts.length - 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 `@docs/source/_static/custom.js` around lines 10 - 13, The text-shortening
logic in custom.js is too broad because it rewrites any dotted literal, which
can incorrectly alter TOC entries like config.yaml or version strings. Update
the handling around the text.includes(".") branch to only truncate API-style
identifiers, using a stricter guard in the same DOM/TOC processing path so
ordinary dotted names and config keys are left unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #385 +/- ##
==========================================
+ Coverage 90.35% 90.52% +0.17%
==========================================
Files 92 92
Lines 11789 11920 +131
Branches 1803 1820 +17
==========================================
+ Hits 10652 10791 +139
+ Misses 794 783 -11
- Partials 343 346 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @elyes298, please do not write custom JS for this task. It is almost certainly unnecessary and definitely not maintainable |
| if not all(str(a.dtype) in NUMPY_TO_VTK_DTYPE for a in cell_data.values()): | ||
| raise ValueError(f"VTK export only supports dtypes {list(NUMPY_TO_VTK_DTYPE.keys())}.") | ||
| return shape | ||
| return shape[0], shape[1], shape[2] |
| current_parent = None | ||
| else: | ||
| current_parent = getattr(current_parent, op) | ||
| current_parent = getattr(current_parent, str(op)) |
| exclude_patterns = [] | ||
|
|
||
| # Hides the class/module nae in the right sidebar | ||
| toc_object_entries_show_parent = 'hide' |
There was a problem hiding this comment.
Should be toc_object_entries_show_parents = 'hide' (you are missing a plural s). Does it work by just setting that option?
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/source/conf.py (1)
104-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the fallback observable
autodoc_preserve_defaultsonly changes how defaults are rendered, so it doesn’t replace this shim;sphinx.util.inspect.object_descriptionis still the hook on the pinned Sphinx line. Keep the fallback only if it still covers a real failure path, but log the exception instead of silently degrading to a generic<ClassName object>/<object>.🤖 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/source/conf.py` around lines 104 - 117, The fallback in _safe_object_description is currently silent, so keep the shim only if it still handles a real failure path but make the exception observable when sphinx.util.inspect.object_description fails. Update the _safe_object_description wrapper in docs/source/conf.py to log or report the caught exception before returning the generic fallback string, while preserving the existing _original_object_description behavior and the sphinx.util.inspect.object_description override.
🤖 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/source/conf.py`:
- Around line 104-117: The fallback in _safe_object_description is currently
silent, so keep the shim only if it still handles a real failure path but make
the exception observable when sphinx.util.inspect.object_description fails.
Update the _safe_object_description wrapper in docs/source/conf.py to log or
report the caught exception before returning the generic fallback string, while
preserving the existing _original_object_description behavior and the
sphinx.util.inspect.object_description override.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c76dc4bb-2cce-44b7-94db-27a9957d4de4
📒 Files selected for processing (1)
docs/source/conf.py
|
|
||
| import sphinx.util.inspect | ||
|
|
||
| _original_object_description = sphinx.util.inspect.object_description |
Comment out pydoclint configuration in pre-commit file.
Removed pydoclint configuration settings.
2 participants
addresses issue #151 which is now solved
Summary by CodeRabbit
pydoclintrules to standardize docstring style and relax specific validation checks.