Skip to content

refactor: factorise output management#38

Merged
HandyS11 merged 2 commits intodevelopfrom
chore/fix-duplication-on-cli
Feb 24, 2026
Merged

refactor: factorise output management#38
HandyS11 merged 2 commits intodevelopfrom
chore/fix-duplication-on-cli

Conversation

@HandyS11
Copy link
Copy Markdown
Owner

This pull request refactors the CLI commands to centralize diagram output handling and standardize output logic across the codebase. The main improvement is the introduction of the DiagramOutputWriter helper, which replaces repeated file and console output logic in each command. This makes the code more maintainable and consistent. The changes also update dependency injection and command constructors to use this new helper.

Centralization and standardization of diagram output:

  • Added DiagramOutputWriter class in src/ProjGraph.Cli/Infrastructure/DiagramOutputWriter.cs, providing reusable methods for writing diagram output to file or console and determining Markdown fence wrapping.
  • Refactored ClassDiagramCommand, ErdCommand, and VisualizeCommand to use DiagramOutputWriter instead of directly handling file and console output. This includes updating constructors and replacing output logic in their ExecuteAsync methods. [1] [2] [3] [4] [5] [6]
  • Updated output wrapping logic in all commands to use DiagramOutputWriter.ShouldWrapInMarkdownFence for consistency. [1] [2] [3]

Dependency injection and program setup:

  • Registered DiagramOutputWriter as a singleton service in the DI container and updated command registration in Program.cs to use named constants for command names and examples.

Codebase simplification:

  • Removed redundant output handling code from each command, reducing duplication and improving maintainability. [1] [2] [3]

Copilot AI review requested due to automatic review settings February 24, 2026 09:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request refactors CLI command output handling by introducing a centralized DiagramOutputWriter helper class. The changes aim to eliminate code duplication and standardize how diagram outputs are written to files or the console across all CLI commands.

Changes:

  • Introduced DiagramOutputWriter class to centralize file and console output logic
  • Updated VisualizeCommand, ErdCommand, and ClassDiagramCommand to use the new helper
  • Registered DiagramOutputWriter in the DI container and introduced command name constants in Program.cs

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/ProjGraph.Cli/Infrastructure/DiagramOutputWriter.cs New helper class that encapsulates diagram output logic including file writing, directory creation, and markdown fence determination
src/ProjGraph.Cli/Program.cs Registers DiagramOutputWriter service and uses constants for command names in examples
src/ProjGraph.Cli/Commands/VisualizeCommand.cs Replaced direct file system usage with DiagramOutputWriter dependency
src/ProjGraph.Cli/Commands/ErdCommand.cs Replaced direct file system usage with DiagramOutputWriter dependency
src/ProjGraph.Cli/Commands/ClassDiagramCommand.cs Replaced direct file system usage with DiagramOutputWriter dependency
Comments suppressed due to low confidence (1)

src/ProjGraph.Cli/Infrastructure/DiagramOutputWriter.cs:48

  • The new implementation changes the behavior for console output (when outputPath is null). The original logic settings.Output?.EndsWith(".mmd", StringComparison.OrdinalIgnoreCase) is false returned false for null inputs (no wrapping), while the new logic outputPath?.EndsWith(".mmd", StringComparison.OrdinalIgnoreCase) is not true returns true for null inputs (wrapping). This means console output will now be wrapped in markdown fences when it wasn't before. If this behavior change is intentional, it should be documented in the PR description. If not, the logic should be changed to outputPath?.EndsWith(".mmd", StringComparison.OrdinalIgnoreCase) is false to preserve the original behavior.
    public static bool ShouldWrapInMarkdownFence(string? outputPath)
    {
        return outputPath?.EndsWith(".mmd", StringComparison.OrdinalIgnoreCase) is not true;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ProjGraph.Cli/Program.cs
Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 24, 2026

@HandyS11 I've opened a new pull request, #39, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: HandyS11 <62420910+HandyS11@users.noreply.github.com>
@HandyS11 HandyS11 merged commit cc814ee into develop Feb 24, 2026
3 checks passed
@HandyS11 HandyS11 deleted the chore/fix-duplication-on-cli branch February 24, 2026 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants