Skip to content

Conversation

aaronmallen
Copy link
Contributor

Introduce @stderr and @stdout instance variables to make error and output streams accessible within commands. This enables more flexible error handling and message output customization.

def perform_command(arguments)
command, args = parse(kommand, arguments, [])

command.instance_variable_set(:@stderr, err)
Copy link
Contributor Author

@aaronmallen aaronmallen Jul 24, 2025

Choose a reason for hiding this comment

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

The idea here is to not have to change the signature of Command#initialize causing a breaking change

Copy link
Contributor

Choose a reason for hiding this comment

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

How about initializing a pair of StringIO in the command, and then

def perform_command(arguments)
  command, args = parse(kommand, arguments, [])
  command.call(**args)
  IO.copy_stream(command.out, out)
  IO.copy_stream(command.err, err)
end

This would make testing the stdout and stderr of commands much simpler.

Copy link
Contributor Author

@aaronmallen aaronmallen Jul 25, 2025

Choose a reason for hiding this comment

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

The intention is to utilize the out/err passed to the CLI in the first place. dry-cil uses them to print help and errors and "inline" commands. We should also allow the commands to use the user specified out/err. Testing is already trivial as the values are DI

@aaronmallen aaronmallen force-pushed the expose-stdout-stderr branch from 914acf9 to 85eb546 Compare July 24, 2025 23:46
Introduce `@stderr` and `@stdout` instance variables to make error
and output streams accessible within commands. This enables more
flexible error handling and message output customization.
@aaronmallen aaronmallen force-pushed the expose-stdout-stderr branch from 85eb546 to 8d8c51d Compare July 25, 2025 03:58
@timriley
Copy link
Member

timriley commented Jul 25, 2025

@aaronmallen I like the idea of this. We already do similar in Hanami CLI, which I think represents good and typical usage of dry-cli:

# in hanami/cli/command.rb

      def initialize(out:, err:, fs:, inflector:)
        super()
        @out = out
        @err = err
        @fs = fs
        @inflector = inflector
      end

I'd love to pull this down into the core library so it's there for everyone who uses it. Since we already make out and err injectable via Dry::CLI#call (good for testing), I think it only makes sense to thread those through to the commands it invoked, and begin setting the expectation that the commands themselves use the provided IO streams, and not manage their own (unless they have good reason to, and in that case, they know they're deviating from the defaults).

In your change here you're conscious to avoid API breakage, but I wonder, perhaps this is worth breaking API for? This feels like a valuable enough thing to move to 2.0 for. And if we did that, is there anything else you'd like to see change with the command interface?

@aaronmallen
Copy link
Contributor Author

aaronmallen commented Jul 25, 2025

And if we did that, is there anything else you'd like to see change with the command interface?

@timriley The only other thing I modify about Dry::CLI locally would actually be a small breaking change. I typically change the example interface to behave like this:

class MyCommand < Dry::CLI::Command
  example "--with-option", "Do a thing with an option"
  example "--without-option", "Do a thing without an option"
end

via

def example(example, description)
  @examples += [example, description].freeze
end

I then implement:

def examples
  indent = @examples.map { |example, _| example.length }.max + 5
  @examples.map { |example, description| "#{example.ljust(indent)}# #{description}" }
end

The idea being I always have a aligned example/description print out:

Examples:

foo --with-option        # Do a thing with an option
foo --without-option     # Do a thing without an option

@aaronmallen
Copy link
Contributor Author

aaronmallen commented Jul 25, 2025

I would even go so far as to say, it would be cool if Dry::CLI::Banner took the width of everything into account and aligned everything in a similar fashion.

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