Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support root commands that doesn't implement call #135

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Support root commands that doesn't implement call #135

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b8567f
Properly display command usage when it has sub-commands and doesn't h…
gustavothecoder Jul 23, 2024
1cef94f
Shows subcommands when root command doesn't implement #call
gustavothecoder Jul 24, 2024
252a7c2
Shows root command help considering if it implements #call
gustavothecoder Jul 24, 2024
3b16959
Create `Dry::CLI::Namespace`, a class to group commands
gustavothecoder Sep 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/dry/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def perform_registry(arguments)
return usage(result) unless result.found?

command, args = parse(result.command, result.arguments, result.names)
return usage(result) unless command.respond_to?(:call)

result.before_callbacks.run(command, args)
command.call(**args)
Expand Down
14 changes: 12 additions & 2 deletions lib/dry/cli/banner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,19 @@ def self.command_name(name)
# @since 0.1.0
# @api private
def self.command_name_and_arguments(command, name)
usage = "\nUsage:\n #{name}#{arguments(command)}"
usage = "\nUsage:\n"

return usage + " | #{name} SUBCOMMAND" if command.subcommands.any?
callable_root_command = false
if command.new.respond_to?(:call)
callable_root_command = true
usage += " #{name}#{arguments(command)}"
end

if command.subcommands.any?
usage += " "
usage += "|" if callable_root_command
usage += " #{name} SUBCOMMAND"
end

usage
end
Expand Down
19 changes: 10 additions & 9 deletions lib/dry/cli/usage.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ def self.call(result)
def self.commands_and_arguments(result)
max_length = 0
ret = commands(result).each_with_object({}) do |(name, node), memo|
args = if node.command && node.leaf? && node.children?
ROOT_COMMAND_WITH_SUBCOMMANDS_BANNER
elsif node.leaf?
arguments(node.command)
else
SUBCOMMAND_BANNER
end

partial = " #{command_name(result, name)}#{args}"
args = arguments(node.command)
args_banner = if node.command && node.leaf? && node.children? && args
ROOT_COMMAND_WITH_SUBCOMMANDS_BANNER
elsif node.leaf? && args
args
elsif node.children?
SUBCOMMAND_BANNER
end

partial = " #{command_name(result, name)}#{args_banner}"
max_length = partial.bytesize if max_length < partial.bytesize
memo[partial] = node
end
Expand Down
12 changes: 12 additions & 0 deletions spec/support/fixtures/shared_commands.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,18 @@ def call(**params)
end
end

class Namespace < Dry::CLI::Command
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't bizarre to have a "command" that the uniq usage is to print top level usage? I am wondering if it should be a better idea to move the usage of top class as specific dsl. Like suggested here #96 ?

Copy link
Author

Choose a reason for hiding this comment

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

It is. I tried to implement the suggestion from #96, but I couldn't figure out how to do it. Maybe we can create the Dry::CLI::Namespace class, so It will look less strange.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good idea.

Copy link
Member

Choose a reason for hiding this comment

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

@gustavothecoder I like the idea of this being a specialised Namespace class. That way, when the class is used, it will be clearer that its purpose is to provide information about a grouping of subcommands only, and not its own distinct command behaviour.

I prefer this help text residing in a concrete class instead of being part of the top-level command registration API, since that feels more consistent with how the rest of the CLI construction is done: a class for each thing.

If you're still interested in working on this PR, I'd love it if you could make this adjustment! Let me know what you think.

Copy link
Author

Choose a reason for hiding this comment

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

@timriley Done! I think it looks better using the Namespace class.

desc "This is a namespace"

class SubCommand < Dry::CLI::Command
desc "I'm a concrete command"

def call(**params)
puts "I'm a concrete command"
end
end
end

class InitializedCommand < Dry::CLI::Command
attr_reader :prop

Expand Down
3 changes: 3 additions & 0 deletions spec/support/fixtures/with_block.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
cli.register "root-command", Commands::RootCommand do |prefix|
prefix.register "sub-command", Commands::RootCommands::SubCommand
end
cli.register "namespace", Commands::Namespace do |prefix|
prefix.register "sub-command", Commands::Namespace::SubCommand
end

cli.register "options-with-aliases", Commands::OptionsWithAliases
cli.register "variadic default", Commands::VariadicArguments
Expand Down
2 changes: 2 additions & 0 deletions spec/support/fixtures/with_registry.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ module Commands
register "with-initializer", ::Commands::InitializedCommand.new(prop: "prop_val")
register "root-command", ::Commands::RootCommand
register "root-command sub-command", ::Commands::RootCommands::SubCommand
register "namespace", ::Commands::Namespace
register "namespace sub-command", ::Commands::Namespace::SubCommand

register "options-with-aliases", ::Commands::OptionsWithAliases
register "variadic default", ::Commands::VariadicArguments
Expand Down
4 changes: 4 additions & 0 deletions spec/support/fixtures/with_zero_arity_block.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
register "root-command" do
register "sub-command", Commands::RootCommands::SubCommand
end
register "namespace", Commands::Namespace
register "namespace" do
register "sub-command", Commands::Namespace::SubCommand
end

register "options-with-aliases", Commands::OptionsWithAliases
register "variadic default", Commands::VariadicArguments
Expand Down
30 changes: 30 additions & 0 deletions spec/support/shared_examples/inherited_commands.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,36 @@
expect(error).to eq(expected)
end

it "shows subcommands when root command doesn't implement #call" do
error = capture_error { cli.call(arguments: %w[namespace]) }
expected = <<~DESC
Commands:
#{cmd} namespace sub-command # I'm a concrete command
DESC
expect(error).to eq(expected)
end

it "shows root command help considering if it implements #call" do
output = capture_output { cli.call(arguments: %w[namespace --help]) }
expected = <<~DESC
Command:
#{cmd} namespace

Usage:
#{cmd} namespace SUBCOMMAND

Description:
This is a namespace

Subcommands:
sub-command # I'm a concrete command

Options:
--help, -h # Print this help
DESC
expect(output).to eq(expected)
end

it "shows run's help" do
output = capture_output { cli.call(arguments: %w[i run --help]) }
expected = <<~DESC
Expand Down
3 changes: 3 additions & 0 deletions spec/support/shared_examples/rendering.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#{cmd} greeting [RESPONSE]
#{cmd} hello # Print a greeting
#{cmd} inherited [SUBCOMMAND]
#{cmd} namespace [SUBCOMMAND] # This is a namespace
#{cmd} new PROJECT # Generate a new Foo project
#{cmd} options-with-aliases # Accepts options with aliases
#{cmd} root-command [ARGUMENT|SUBCOMMAND] # Root command with arguments and subcommands
Expand Down Expand Up @@ -80,6 +81,7 @@
#{cmd} greeting [RESPONSE]
#{cmd} hello # Print a greeting
#{cmd} inherited [SUBCOMMAND]
#{cmd} namespace [SUBCOMMAND] # This is a namespace
#{cmd} new PROJECT # Generate a new Foo project
#{cmd} options-with-aliases # Accepts options with aliases
#{cmd} root-command [ARGUMENT|SUBCOMMAND] # Root command with arguments and subcommands
Expand Down Expand Up @@ -109,6 +111,7 @@
#{cmd} greeting [RESPONSE]
#{cmd} hello # Print a greeting
#{cmd} inherited [SUBCOMMAND]
#{cmd} namespace [SUBCOMMAND] # This is a namespace
#{cmd} new PROJECT # Generate a new Foo project
#{cmd} options-with-aliases # Accepts options with aliases
#{cmd} root-command [ARGUMENT|SUBCOMMAND] # Root command with arguments and subcommands
Expand Down