-
Notifications
You must be signed in to change notification settings - Fork 0
Fix abbreviation casing issues #254
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
Conversation
Reviewer's Guide by SourceryThis pull request addresses casing and formatting inconsistencies in headings across multiple MDX files. It updates the Updated class diagram for format-headings.tsclassDiagram
class format-headings.ts {
<<file>>
+restoreAbbreviations()
+toSentenceCase(text: string): string
+processLine(line: string): string
}
note for format-headings.ts "The toSentenceCase function now removes any remaining asterisks instead of handling markdown formatting."
note for format-headings.ts "The processLine function now uses a regex to handle all numbering patterns, but preserve existing formatting."
File-Level Changes
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 @gyan-sharma - I've reviewed your changes - here's some feedback:
Overall Comments:
- The changes look good, but the regular expression in
format-headings.tscould be simplified. - Consider adding a test case for the
format-headings.tsfile to ensure the logic works as expected.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 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.
| } | ||
|
|
||
| // Function to process a single line | ||
| function processLine(line: string): string { |
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.
issue (complexity): Consider extracting heading parsing and reconstruction into helper functions to simplify the processLine function and improve readability and testability without altering functionality.
Consider extracting the heading parsing and reconstruction into small helper functions. This breaks the complexity in processLine down into focused responsibilities, making the code easier to read and test without altering functionality.
For example, you can start by creating a helper to parse headings:
function parseHeading(line: string) {
const headingRegex =
/^(#{1,6})\s*(\*\*)?(?:(\d{1,2}\.(?:\d{1,2})?)\.\s*)?([a-zA-Z].*?)(\*\*)?$/;
const match = line.match(headingRegex);
if (!match) return null;
const [_, hashes, startBold, numbers, content, endBold] = match;
return { hashes, startBold, numbers, content, endBold };
}Then refactor processLine to delegate parsing and formatting:
function processLine(line: string): string {
if (!line.startsWith("#")) {
return line;
}
const parsed = parseHeading(line);
if (!parsed) return line;
const { hashes, startBold, numbers, content, endBold } = parsed;
const processedContent = content.charAt(0).toUpperCase() + content.slice(1);
const header = numbers ? `${hashes} ${numbers}. ` : `${hashes} `;
if (startBold) {
return header + startBold + processedContent + (endBold || "**");
}
return header + processedContent;
}This approach reduces nesting and makes each part easier to manage.
Summary by Sourcery
Fix abbreviation and casing issues in documentation and code formatting
Enhancements:
Documentation: