Skip to content

Conversation

@cawpea
Copy link
Owner

@cawpea cawpea commented Jan 2, 2026

No description provided.

cawpea and others added 11 commits January 2, 2026 12:13
- Add message-formatter.ts for consistent CLI output styling
- Replace raw ANSI codes with chalk in diff-display.ts
- Standardize emoji usage across all commands (10 distinct types)
- Implement color scheme (red=error, green=success, yellow=warning, cyan=info)
- Translate Japanese comments/messages to English in backup.ts
- Refactor all console output to use msg.* helpers
- Add verbose mode support with debug messages

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add displayColoredCodeBlock function to parse markdown code blocks
- Display code with green + prefix and dim backticks
- Apply to INSERT_CODE_BLOCK and default preview cases
- Improve visual clarity when previewing code insertions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Detect CODE_REF comment changes with arrow (→)
- Display old CODE_REF in red, arrow in yellow, new CODE_REF in green
- Highlight CODE_REF: label in cyan bold for single comments
- Dim HTML comment markers (<!-- and -->)
- Improve visual clarity when reviewing fix options

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…NTENT_MISMATCH

When Option 2 is selected for CODE_CONTENT_MISMATCH errors (converting symbol reference to line numbers),
the preview now displays a diff showing:
- The code block that will be kept (from markdown)
- The actual code in the file at the new line numbers

This makes it clear to users that the code block needs manual adjustment after updating the CODE_REF comment
to use line numbers instead of symbol references.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ISMATCH

When Option 2 is selected for CODE_CONTENT_MISMATCH errors (converting symbol
reference to line numbers), the preview was incorrectly showing both expected
and actual code as the same.

Root cause: The switch statement was using error.type instead of action.type,
which caused it to hit the CODE_CONTENT_MISMATCH case handler instead of the
UPDATE_LINE_NUMBERS case handler. This resulted in comparing error.expectedCode
(1 line from document) with action.newCodeBlock (also 1 line from document)
instead of extracting the actual code from the file (4 lines with JSDoc).

Changes:
- Switch on action.type instead of error.type in displayFixPreview
- Add special handling for CODE_LOCATION_MISMATCH within UPDATE_LINE_NUMBERS case
- Remove unused displayLineRangeDiff import
- Update test to use flexible string matching for colored output

Now correctly displays:
- Expected code (in document): 1 line without JSDoc
- Actual code (in file): 4 lines with JSDoc comment

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
When Option 2 is selected for CODE_CONTENT_MISMATCH (convert symbol to line numbers),
the previous behavior showed a confusing diff between document code and file code,
making users think the code block would be updated. However, Option 2 only updates
the CODE_REF comment and keeps the code block unchanged.

Changes:
- Remove code diff display for UPDATE_LINE_NUMBERS from CODE_CONTENT_MISMATCH
- Show clear warning message that code block will remain unchanged
- Remove unused extractLinesFromFile import

This makes it clear that:
- Option 1: Updates both CODE_REF and code block
- Option 2: Updates only CODE_REF, code block needs manual adjustment

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
  - Add note about color-coded output in CLI usage guide
  - Describe standardized color schemes and emoji indicators
  - Update following introduction of centralized message formatter
Add 48 test cases covering:
- Basic message formatting (error, warning, info, success, neutral)
- Verbose mode and debug functionality
- Error detail formatting with optional location
- Summary formatting with backup paths
- Specialized messages (startFix, startValidation, file, backup, skip, context)
- msg convenience export
- COLOR_SCHEMES export
- Special character handling (HTML, newlines, Unicode)
- Edge cases (long messages, large numbers, many backups)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Jan 2, 2026

PR Review: Release - Update Style of Prompts for Consistency

Summary

This PR introduces a centralized message formatting system to standardize CLI output across the validate and fix commands. The changes significantly improve user experience through consistent color schemes, emoji usage, and structured output formatting.

Code Quality & Best Practices ✅

Strengths

  1. Excellent Separation of Concerns: The new message-formatter.ts provides a clean abstraction for all CLI messaging, following the Single Responsibility Principle.

  2. Consistent API Design: The msg convenience export provides a clean, functional API that's easy to use throughout the codebase.

  3. Well-Tested: The addition of 382 lines of comprehensive tests for message-formatter.test.ts shows strong commitment to quality (48 test cases covering edge cases, special characters, and various message types).

  4. Good Documentation: JSDoc comments are thorough and helpful, explaining parameters and return values clearly.

  5. Internationalization: Translation of Japanese comments to English in backup.ts improves maintainability for international contributors.

  6. Color Scheme Consistency: Using centralized COLOR_SCHEMES export ensures colors are consistent across different output utilities (diff-display.ts, styles.ts).

Code Style Adherence ✅

The code follows all standards from CLAUDE.md:

  • ✅ 2-space indentation
  • ✅ Semicolons enabled
  • ✅ Single quotes
  • ✅ Type imports used appropriately
  • ✅ 100 character line width respected

Potential Issues & Concerns

1. Static State in MessageFormatter (Minor)

Location: src/utils/message-formatter.ts:49

export class MessageFormatter {
  private static verboseMode = false;

Issue: Using static state for verboseMode could cause issues in testing or if the module is used in multiple contexts. While this is likely fine for a CLI tool, it's worth noting.

Suggestion: Consider passing verbose mode as a parameter to methods that need it, or document that this is intentional for CLI usage.

2. Missing Type Safety for Emojis (Minor)

Location: src/utils/message-formatter.ts:32-43

The EMOJIS object is not exported, which is fine, but some emojis defined aren't used by any public methods (e.g., skip, backup have dedicated methods, but search, fix, file, stats are only used internally).

Suggestion: This is actually well-designed as-is. No action needed.

3. Regex Efficiency in formatCodeLine (Minor)

Location: src/utils/styles.ts:176-198

The function creates multiple regex instances on every line formatting call:

keywords.forEach((keyword) => {
  const regex = new RegExp(`\\b(${keyword})\\b`, 'g');
  formattedLine = formattedLine.replace(regex, chalk.cyan('$1'));
});

Impact: Low - this is for CLI output which is not performance-critical.

Suggestion: For micro-optimization, pre-compile regexes as constants outside the function.

4. Code Duplication in Preview Display (Minor)

Location: src/utils/prompt.ts:113-186

The displayFixPreview function has grown quite large with multiple switch cases. Some logic could be extracted into helper functions.

Suggestion: Consider extracting helpers like displayInsertionPreview(), displayReplacementPreview(), etc. for better maintainability.

Performance Concerns

No significant performance concerns. All changes are in CLI output formatting which is inherently I/O bound and not performance-critical.

Security Concerns

No security issues identified. The changes are primarily cosmetic and don't introduce:

  • User input handling vulnerabilities
  • File system security issues
  • Injection vulnerabilities

The code properly escapes special characters in regex patterns and uses chalk for safe terminal output.

Test Coverage

Strengths ✅

  1. Comprehensive Coverage: 48 test cases added for message-formatter.ts covering:

    • Basic formatting (error, warning, info, success, neutral)
    • Verbose mode toggle
    • Error details with locations
    • Summary formatting
    • Edge cases (empty strings, special characters, HTML, Unicode)
  2. Updated Existing Tests: Tests in diff-display.test.ts updated to handle colored output.

Gaps ⚠️

  1. Integration Testing: No integration tests verifying the actual CLI output in validate and fix commands with the new formatter.

  2. Visual Testing: No snapshot tests to ensure formatting remains consistent across changes.

Recommendation: Consider adding snapshot tests for full CLI output examples to catch unintended formatting changes.

Documentation

Strengths ✅

  1. Updated docs/user-guide/cli-usage.md to document the new color-coded output.
  2. Clear commit messages following Conventional Commits standard.

Missing ⚠️

  1. Migration Guide: No documentation about the changes for existing users (though this is cosmetic and shouldn't break anything).

  2. Color Scheme Reference: Consider documenting the full color scheme somewhere for reference:

    • Red: Errors
    • Yellow: Warnings
    • Green: Success
    • Cyan: Info
    • Gray: Debug/Dim

Recommendation: Add a section to the CLI usage guide showing the color scheme or include it in README.

Architectural Considerations

Strengths ✅

  1. Centralization: Moving from scattered ANSI codes to a centralized formatter is a significant architectural improvement.

  2. Extensibility: The design makes it easy to add new message types or change color schemes globally.

  3. Backwards Compatibility: Changes are internal-only and don't affect the public API.

Observations

  1. Diff Display Grouping: The change to group diff changes by type (commit 3f891eb) improves readability significantly. The implementation correctly buffers removed/added lines and flushes them in the right order.

Specific Code Review Comments

src/utils/message-formatter.ts

Line 226-237: The msg convenience export is well-designed, but consider adding a type definition:

export interface MessageFormatterAPI {
  error: (text: string) => string;
  warning: (text: string) => string;
  // ... etc
}

export const msg: MessageFormatterAPI = { /* ... */ };

This would provide better autocomplete and type safety.

src/utils/diff-display.ts

Line 46-61: The change buffering logic is clever:

if (removedLines.length > 0 || addedLines.length > 0) {
  flushChanges();
}

However, this could be extracted to a helper method for clarity:

private flushBufferedChanges() {
  if (this.removedLines.length > 0 || this.addedLines.length > 0) {
    this.flushChanges();
  }
}

src/cli/fix.ts

Line 90: Good addition of verbose mode support:

msg.setVerbose(options.verbose);

Line 217: The verbose check is correct:

if (lineDelta !== 0 && options.verbose) {

However, since msg.debug() already checks verbose mode internally, this could be simplified to:

if (lineDelta !== 0) {
  console.log(msg.debug(`   Line delta: ${lineDelta > 0 ? '+' : ''}${lineDelta}`));
}

Overall Assessment

✅ Approve with Minor Suggestions

This is a high-quality PR that significantly improves the user experience of the CLI tool. The changes are:

  1. Well-architected and maintainable
  2. Thoroughly tested
  3. Properly documented
  4. Follow coding standards
  5. Don't introduce security vulnerabilities
  6. Have no significant performance impact

Priority Recommendations

Before Merge (Optional but Recommended):

  1. Add snapshot tests for CLI output examples
  2. Document the color scheme in user-facing docs

Post-Merge (Nice to Have):

  1. Consider extracting large switch statements in displayFixPreview to helper functions
  2. Pre-compile regex patterns in formatCodeLine for micro-optimization
  3. Add type interface for msg export

Testing Recommendation

Before merging, please verify:

  1. ✅ All unit tests pass: npm test
  2. ✅ Linting passes: npm run lint
  3. ✅ Type checking passes: npm run type-check
  4. ✅ Demo validation works: npm run demo:validate:invalid (should show colored output)

Great work on improving the developer experience! The centralized formatting system will make future maintenance much easier. 🎉

@cawpea cawpea merged commit 84e4fe5 into main Jan 2, 2026
6 of 7 checks passed
@github-actions
Copy link

github-actions bot commented Jan 2, 2026

🎉 This PR is included in version 0.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants