-
-
Notifications
You must be signed in to change notification settings - Fork 23
Fix bad anchors #132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix bad anchors #132
Conversation
Reviewer's Guide by SourceryThis pull request fixes several issues related to anchor generation in mkdoxy, specifically addressing problems with overloaded operators, special characters in names, and the escaping of special characters in markdown. The changes ensure that anchors are correctly generated for a wider range of cases, improving navigation and linking within the generated documentation. Updated class diagram for Node classclassDiagram
class Node {
- _name: str
- _refid: str
- _visibility: Visibility
+ operator_num: int
+ name_url_safe: str
+ anchor: str
+ operators_total() int
}
note for Node.name_url_safe "Strips special characters (=~.,<>) from name"
note for Node.anchor "Generates correct anchor name for overloaded operators and other cases"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @EmilyBourne - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider consolidating the replace calls in
name_url_safe
into a single regex replace for better readability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @EmilyBourne - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a unit test to verify the generated anchors for template syntax.
- The logic for handling overloaded operators seems complex; can it be simplified?
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @EmilyBourne - I've reviewed your changes - here's some feedback:
Overall Comments:
- It might be worth adding a unit test to verify the anchor generation logic, especially for edge cases.
- Consider adding a comment explaining the regex in
name_url_safe
.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This PR should be ok now. Is there anywhere to add tests? If not then I can at least confirm that it runs correctly on the repo that I am working with (see gyselax/gyselalibxx#193 ) and removes all the warnings that I was seeing, without creating any new warnings. |
Can you please create a PR for #126 or after merging it? More info there: #135 (comment) Thanks |
Fix bad anchors created for:
operator()
(missing fromOVERLOAD_OPERATORS
)-
as this character is preserved in the anchorMyClass<Specialisation, Specialization>
)...
private
zone (where the documentation is not extracted by doxygen)This fixes #131
Summary by Sourcery
Improve anchor generation for documentation to handle complex operator and section naming scenarios
Bug Fixes:
operator()
Enhancements:
Chores:
operator()
to the list of recognized overload operators