Improve output summary#2
Merged
Merged
Conversation
Updated output messages to provide detailed summaries, including counts of files with issues or cleaned files. Improved clarity and consistency in the display of file check results. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughBumps package to 1.3.1-beta, updates formatters to emit combined checked/clean summary lines (including broken-link counts and corrected pluralization), defers CLI printing into numbered or unnumbered sections via a ChangesEnhanced Output Summary Formatting & CLI Sectioning
Sequence Diagram(s)sequenceDiagram
participant formatValidationResult
participant formatDeprecationResult
participant formatLinkCheckResult
participant formatMinificationResult
participant filesSummary
participant CLISections as "bin/hihtml.js:sections"
participant Console
formatValidationResult->>filesSummary: request checked/clean summary
formatDeprecationResult->>filesSummary: request checked/clean summary
formatLinkCheckResult->>filesSummary: request checked/clean + broken count
formatMinificationResult->>filesSummary: request filesCount string
filesSummary->>CLISections: return formatted section strings
CLISections->>Console: emit numbered/unnumbered sections
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 11: Update the changelog entry to mention the link-check output summary
enhancement in addition to validation and deprecated markup checks: reference
that formatLinkCheckResult (in src/lib/output.js) now includes broken link
counts with singular/plural handling so the entry describes all three modified
formatters (formatValidationResult, formatDeprecatedMarkupResult, and
formatLinkCheckResult).
In `@src/lib/output.js`:
- Around line 169-171: The summaryBroken computation uses result.countBroken ===
0 && result.countFileErrors === 0 to decide success styling, causing a warning
style when countBroken === 0 but countFileErrors > 0; update the logic in
summaryBroken so it only checks result.countBroken (i.e. treat zero broken links
as success regardless of file errors) and call s.success when result.countBroken
=== 0, otherwise s.warning with the count and pluralization; modify the code
that sets summaryBroken (and not summaryFileErrors) so file-error presence no
longer forces warning styling for broken-links.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a724d15-e114-46d7-94af-bef6c70add5a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
CHANGELOG.mdpackage.jsonsrc/lib/output.js
Replaced repetitive file summary logic with reusable helper functions (`filesCount` and `filesSummary`). This improves code readability, reduces redundancy, and ensures consistent output formatting across all file validation and reporting functions. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Revised the output generation logic to consolidate sections into arrays, ensuring cleaner and more modular handling of outputs. Added functionality to number sections when multiple are displayed, improving readability and clarity in reports. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Adjusted the regex in the output formatting logic to account for cases with double newlines before summaries. This ensures consistent handling of titles and summaries, improving overall output clarity. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bin/hihtml.test.js`:
- Around line 395-403: The "Numbers all sections" test is nested inside another
test callback; cut the entire test block starting with test('Numbers all
sections', ...) and paste it as a top-level sibling test (i.e., not inside any
other test() callback) so it runs independently under node:test; keep the
existing assertions (assert.ok checks), the call to run([...]) and the cleanup
fs.rmSync call, and ensure it references the same helpers/variables (run,
tempDir, path) as before.
In `@src/lib/output.js`:
- Around line 153-155: The summary currently counts all entries in withIssues
(used to build fileParts) which includes files that only had skipped links;
change the logic so the summary counts only true-issue files by replacing use of
withIssues.length with a filtered count (e.g., count entries in withIssues that
are not skipped-only) before pushing the warning into fileParts; keep cleanCount
and fileParts as-is but reference the filtered issues count when creating the
`${...} with issues` message (use the existing withIssues identifier and the
property that marks skipped-only files, e.g., skippedOnly or
hasOnlySkippedLinks).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e994f488-3391-4f68-a499-c1328117088b
📒 Files selected for processing (4)
CHANGELOG.mdbin/hihtml.jsbin/hihtml.test.jssrc/lib/output.js
Relocated the "Numbers all sections" test to follow the correct cleanup logic within the test suite. This improves structure and ensures tests run in the intended sequence without unnecessary duplication. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Replaced `cleanCount` with `countClean` and introduced `countIssues` to enhance clarity and consistency in file issue reporting. Updated corresponding output logic for better readability and maintainability. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/output.js (1)
99-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix clean-count baseline in link summary.
Line 102 computes
countCleanfromwithIssues.length, butwithIssuesincludes skipped-only files. That makes skipped-only files count as neither “with issues” nor “clean”, and can produce an empty middle summary segment on Line 163.💡 Minimal fix
- const countClean = result.files.length - withIssues.length; + const countClean = result.files.length - countIssues;Also applies to: 156-163
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/output.js` around lines 99 - 103, The clean-count calculation is wrong because withIssues includes skipped-only files; compute countIssues as you already do, then compute countSkipped (e.g., from result.files.filter(f => f.skipped).length) and set countClean = result.files.length - countIssues - countSkipped; update the summary rendering that uses countClean (the link summary segment generation) so it uses this corrected countClean value instead of subtracting withIssues.length.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/lib/output.js`:
- Around line 99-103: The clean-count calculation is wrong because withIssues
includes skipped-only files; compute countIssues as you already do, then compute
countSkipped (e.g., from result.files.filter(f => f.skipped).length) and set
countClean = result.files.length - countIssues - countSkipped; update the
summary rendering that uses countClean (the link summary segment generation) so
it uses this corrected countClean value instead of subtracting
withIssues.length.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c33f1ba-b84c-4932-8cba-6c8699982d70
📒 Files selected for processing (2)
bin/hihtml.test.jssrc/lib/output.js
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit