Conversation
|
WalkthroughThe GitHub Actions workflow script was updated to improve the detection of debug statements in source files by ensuring that only non-commented instances are flagged. This was achieved by modifying the grep commands to exclude lines that are commented out, while the overall workflow logic and control flow remain unchanged. Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
![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
♻️ Duplicate comments (3)
.github/workflows/npm-publish.yml (3)
84-91: Duplicate: Apply comment-exclusion improvements here
The same enhancements (HTML comment support, broader block-comment exclusion, and DRYing the regex) suggested above for$inspectapply equally to theconsole.logcheck.
93-100: Duplicate: Apply comment-exclusion improvements here
Please update theconsole.debugcheck with the sharedIGNORE_COMMENTSpattern as outlined above to cover HTML comments and block comments consistently.
102-109: Duplicate: Apply comment-exclusion improvements here
Likewise, thedebugger;check should leverage the sameIGNORE_COMMENTSvariable to avoid false positives in commented-out code.
| # Check for $inspect (not in comments) | ||
| if grep -n -P '^(?!\s*//|\s*/\*|\s*\*).*\$inspect' "$file" > /dev/null; then | ||
| match=$(grep -n -P '^(?!\s*//|\s*/\*|\s*\*).*\$inspect.*' "$file" | head -n 1) | ||
| line_num=$(echo "$match" | cut -d: -f1) | ||
| statement=$(echo "$match" | cut -d: -f2-) | ||
| echo "::error file=$file::Found \$inspect statement on line $line_num: $statement" | ||
| has_debug=true | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance comment exclusion and DRY the regex
The negative lookahead currently filters out lines starting with //, /*, or *, but:
- It doesn’t exclude HTML comment markers (
<!--) used in.sveltefiles, so commented-out debug code in HTML comments will still trigger false positives. - It only skips lines where the comment marker is at the very start (after whitespace), but block comments without leading
*on inner lines (e.g.,/*\n console.log...\n*/) can still slip through. - The same long regex is duplicated four times.
Proposed diff to address these and reduce duplication:
@@ check_file() {
- # Check for $inspect (not in comments)
- if grep -n -P '^(?!\s*//|\s*/\*|\s*\*).*\$inspect' "$file" > /dev/null; then
+ # Define pattern to ignore JavaScript and HTML comment lines
+ local IGNORE_COMMENTS='^(?!\s*(?://|/\*|\*|<!--))'
+
+ # Check for $inspect (not in comments)
+ if grep -n -P "${IGNORE_COMMENTS}.*\\\$inspect" "$file" > /dev/null; then
match=$(grep -n -P '^(?!\s*//|\s*/\*|\s*\*).*\$inspect.*' "$file" | head -n 1)
line_num=$(echo "$match" | cut -d: -f1)
statement=$(echo "$match" | cut -d: -f2-)You can then replace the other three grep calls to use ${IGNORE_COMMENTS} instead of repeating the full pattern. This will ensure HTML comments are respected and make future updates simpler.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/npm-publish.yml around lines 75 to 82, the regex used to
exclude comment lines when searching for $inspect statements does not account
for HTML comment markers like <!-- and only excludes comment markers at the
start of the line, missing inner block comment lines. Additionally, the same
complex regex is repeated multiple times. To fix this, define a variable (e.g.,
IGNORE_COMMENTS) that includes an improved negative lookahead pattern covering
//, /*, *, and <!-- comment styles and use this variable in all grep commands to
avoid duplication and ensure all comment types are properly excluded.
1 participant
Summary by CodeRabbit