-
Notifications
You must be signed in to change notification settings - Fork 17
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
https://github.com/jackdewinter/pymarkdown/issues/1280 #1281
Conversation
Reviewer's Guide by SourceryThis PR implements fixes for task list handling in Rule MD018 and adds functionality to capture test cases in a specified directory. The changes include adding task list token support, modifying test infrastructure, and updating dependencies. Sequence diagram for test case directory capturesequenceDiagram
participant User
participant Script
participant FileSystem
User->>Script: Run ptest.cmd with -d option
Script->>FileSystem: Check if directory exists
alt Directory exists
Script->>FileSystem: Change to specified directory
Script->>FileSystem: Erase .md files
else Directory does not exist
Script->>User: Display error message
end
Updated class diagram for MarkdownTokenclassDiagram
class MarkdownToken {
- _token_task_list: str
- _token_front_matter: str
- _token_block_quote: str
- _token_unordered_list_start: str
- _token_ordered_list_start: str
- _token_new_list_item: str
+ is_task_list() bool
}
note for MarkdownToken "Added is_task_list property and reordered token constants"
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 @jackdewinter - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues 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.
@@ -1 +1,5 @@ | |||
pipenv run python main.py %1 %2 %3 %4 %5 %6 %7 %8 %9 | |||
set RETURN_CODE=%ERRORLEVEL% | |||
echo !RETURN_CODE! | |||
echo %RETURN_CODE% |
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 (bug_risk): The %RETURN_CODE% reference won't show the actual return code since it's not using delayed expansion
Use !RETURN_CODE! consistently since delayed expansion is needed for variables set within the script
) | ||
|
||
set xx= | ||
erase %XX_TEMPFILE% |
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 (bug_risk): Temp file cleanup won't happen if script exits early due to an error
Consider implementing cleanup in an error handler block to ensure the temp file is always removed
) else ( | ||
pushd %OLDDIR% | ||
cd | ||
cd !PTEST_KEEP_DIRECTORY! |
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: Directory path handling doesn't properly handle paths containing spaces
Wrap !PTEST_KEEP_DIRECTORY! in quotes to handle paths containing spaces
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1281 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 191 191
Lines 21390 21393 +3
Branches 2751 2751
=========================================
+ Hits 21390 21393 +3 ☔ View full report in Codecov by Sentry. |
#1280
Summary by Sourcery
Introduce a new feature to save and scan Markdown documents with specific extensions enabled, fix a bug related to task lists in rule Md018, and enhance the testing infrastructure with a new test case and batch script. Update the changelog and reorganize the Pipfile dependencies.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: