-
-
Notifications
You must be signed in to change notification settings - Fork 500
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
fixes bug in talawa_mobile_md_mdx_format_adjuster.py #2695
fixes bug in talawa_mobile_md_mdx_format_adjuster.py #2695
Conversation
WalkthroughThe changes involve removing the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
Other🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise. |
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.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/talawa_mobile_md_mdx_format_adjuster.py (1)
79-79
: Remove or justify the commented-out function call.If escaping special characters is no longer necessary, deleting this call (rather than leaving it commented) clarifies the intended logic. Alternatively, if there's a future plan to reintroduce special character escaping, consider adding a comment explaining why this remains commented out.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/talawa_mobile_md_mdx_format_adjuster.py
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Checking codebase
🔇 Additional comments (1)
.github/workflows/talawa_mobile_md_mdx_format_adjuster.py (1)
21-42
: Consider removing the commented-outescape_mdx_characters
function entirely.Currently, the function is fully commented out. If it is no longer needed, removing it would reduce clutter and improve maintainability. However, confirm that no other workflow or script relies on escaping special characters before final removal.
You can run the following script to search usage references across the codebase:
✅ Verification successful
Remove the commented-out
escape_mdx_characters
functionThe function is unused and can be safely removed. The MDX processing in the repository is handled by Docusaurus's built-in MDX processor, making this custom escaping function unnecessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the function name appears anywhere else in the repository rg -A 5 'escape_mdx_characters'Length of output: 31
Script:
#!/bin/bash # Search with case-insensitive flag and include comments rg -i 'escape.*mdx|mdx.*escape' --type py # Find all MDX-related files fd -e mdx # Search for similar MDX character escaping patterns rg -i 'escape.*(characters?|[<>{}])' --type py # Look for any MDX processing files fd -g '*mdx*.py'Length of output: 508
Script:
#!/bin/bash # Find all Python files fd -e py # Check content of MDX files for special characters that might need escaping cat docs/blog/2021-08-01-mdx-blog-post.mdx docs/docs/tutorial-basics/markdown-features.mdx # Search in Python files with explicit extension rg -i --type py 'escape|mdx|[<>{}]'Length of output: 3808
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2695 +/- ##
=================================================
Coverage 96.45% 96.45%
=================================================
Files 189 189
Lines 9994 9994
=================================================
Hits 9640 9640
Misses 354 354 ☔ View full report in Codecov by Sentry. |
def escape_mdx_characters(text): | ||
""" | ||
Escape special characters (<, >, {, }) in Dart docs to make them MDX compatible. | ||
|
||
Args: | ||
text (str): The text content to be processed. | ||
|
||
Returns: | ||
str: The modified string with escaped MDX characters. | ||
""" | ||
# Replace unescaped <, >, {, } with their escaped equivalents | ||
patterns = { | ||
"<": r"(?<!\\)<", | ||
">": r"(?<!\\)>", | ||
"{": r"(?<!\\){", | ||
"}": r"(?<!\\)}" | ||
} | ||
# def escape_mdx_characters(text): | ||
# """ | ||
# Escape special characters (<, >, {, }) in Dart docs to make them MDX compatible. | ||
|
||
# Args: | ||
# text (str): The text content to be processed. | ||
|
||
# Returns: | ||
# str: The modified string with escaped MDX characters. | ||
# """ | ||
# # Replace unescaped <, >, {, } with their escaped equivalents | ||
# patterns = { | ||
# "<": r"(?<!\\)<", | ||
# ">": r"(?<!\\)>", | ||
# "{": r"(?<!\\){", | ||
# "}": r"(?<!\\)}" | ||
# } |
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.
Is just commenting the code fixes the issue ??
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.
Yes it will. Try it once....
This part of code is responsible for converting to escaped characters.
Once sir peter commented out the script and when we ran it there were no more escaped characters.
Closing. You are not assigned the issue |
@palisadoes sir i was assigned to this issue. |
What kind of change does this PR introduce?
bugfix
Issue Number:#2685
Fixes #2685
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
yes
Summary by CodeRabbit