Skip Menu |

This queue is for tickets about the CLI-Dispatch CPAN distribution.

Report information
The Basics
Id: 83019
Status: resolved
Priority: 0/
Queue: CLI-Dispatch

People
Owner: Nobody in particular
Requestors: djerius [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.16
Fixed in: (no value)



Subject: CLI::Dispatch::Help searches too deep
Hi, Thanks for writing CLI::Dispatch. I've run across a small problem with CLI::Dispatch::Help::list_commands. I'm attempting to create "sub" commands, as in: bin/x b c where c is a subcommand for b. This is working quite well, actually. Below is a layout of the code (hopefully this survives reformatting) . ├── bin │ └── x └── lib ├── X │ ├── A.pm │ ├── B │ │ ├── X │ │ │ └── C.pm │ │ └── X.pm │ └── B.pm └── X.pm and B.pm is a mix of a command and a top level dispatcher: package X::B; use CLI::Dispatch; use base 'CLI::Dispatch::Command'; use X::B::X; sub run { X::B::X->run; } 1; The only problem that I've come across so far is that CLI::Dispatch::Help::list_commands searches deep into the namespaces, and finds the sub-commands when I believe it shouldn't, with odd results: % bin/x a - x::a b - x::b c - [disabled: compile error] help - show help x - [disabled: compile error] It's finding X::B::X.pm and X::B::X::C.pm when it shouldn't. X::B::X.pm isn't a command at all, and X::B::X::C.pm is b's c command. I expected something like this: % bin/x a - x::a b - x::b help - show help With help for b's subcommands available when I run % bin/x b c - x b x c help - show help (that does work fine). It doesn't look like CLI::Dispatch::Help is performing the deep search for any real reason; it's not attempting to build a list of sub commands (as %found is keyed off of the basename of the module, not the relative path to the top of the namespace). Is the deep search necessary? I realize that what I'm doing isn't using the documented behavior, but the implementations of sub-commands is so straightforward that it'd be a shame not to be able to use it. Thanks, Diab P.S. I've attached a zip file of the example code.
Subject: cli-dispatch.zip
Download cli-dispatch.zip
application/zip 2.5k

Message body not shown because it is not plain text.

I've attached a patch against 0.16 to stop the deep directory searches. My simple scheme for constructing sub-commands doesn't pass along global command options, unfortunately.
Subject: CLI-Dispatch-0.16-recurse.patch
# This is a patch for CLI-Dispatch-0.16.orig to update it to CLI-Dispatch-0.16 # # To apply this patch: # STEP 1: Chdir to the source directory. # STEP 2: Run the 'applypatch' program with this patch file as input. # # If you do not have 'applypatch', it is part of the 'makepatch' package # that you can fetch from the Comprehensive Perl Archive Network: # http://www.perl.com/CPAN/authors/Johan_Vromans/makepatch-x.y.tar.gz # In the above URL, 'x' should be 2 or higher. # # To apply this patch without the use of 'applypatch': # STEP 1: Chdir to the source directory. # STEP 2: Run the 'patch' program with this file as input. # #### End of Preamble #### #### Patch data follows #### diff -c 'CLI-Dispatch-0.16.orig/lib/CLI/Dispatch/Help.pm' 'CLI-Dispatch-0.16/lib/CLI/Dispatch/Help.pm' Index: ./lib/CLI/Dispatch/Help.pm *** ./lib/CLI/Dispatch/Help.pm Sat Nov 5 03:56:57 2011 --- ./lib/CLI/Dispatch/Help.pm Tue Jan 29 14:12:26 2013 *************** *** 86,94 **** foreach my $path ( @paths ) { my $dir = dir( $inc, $path ); next unless $dir->exists; ! $dir->recurse( callback => sub { ! my $file = shift; ! return if $file->is_dir; my $basename = $file->basename; $basename =~ s/\.(?:pm|pod)$//; --- 86,95 ---- foreach my $path ( @paths ) { my $dir = dir( $inc, $path ); next unless $dir->exists; ! ! for my $file ( $dir->children ) { ! ! next if $file->is_dir; my $basename = $file->basename; $basename =~ s/\.(?:pm|pod)$//; *************** *** 99,105 **** $class .= '::'.$basename; # ignore base class ! return if $class eq 'CLI::Dispatch::Command'; my $podfile = $file->parent->file($basename . '.pod'); my $pmfile = $file->parent->file($basename . '.pm'); --- 100,106 ---- $class .= '::'.$basename; # ignore base class ! next if $class eq 'CLI::Dispatch::Command'; my $podfile = $file->parent->file($basename . '.pod'); my $pmfile = $file->parent->file($basename . '.pm'); *************** *** 134,140 **** my $len = length $basename; $maxlength = $len if $maxlength < $len; ! }); } } --- 135,141 ---- my $len = length $basename; $maxlength = $len if $maxlength < $len; ! } } } #### End of Patch data #### #### ApplyPatch data follows #### # Data version : 1.0 # Date generated : Tue Jan 29 14:16:37 2013 # Generated by : makepatch 2.04 # Recurse directories : Yes # Excluded files : (\A|/).*\~\Z # (\A|/).*\.a\Z # (\A|/).*\.bak\Z # (\A|/).*\.BAK\Z # (\A|/).*\.elc\Z # (\A|/).*\.exe\Z # (\A|/).*\.gz\Z # (\A|/).*\.ln\Z # (\A|/).*\.o\Z # (\A|/).*\.obj\Z # (\A|/).*\.olb\Z # (\A|/).*\.old\Z # (\A|/).*\.orig\Z # (\A|/).*\.rej\Z # (\A|/).*\.so\Z # (\A|/).*\.Z\Z # (\A|/)\.del\-.*\Z # (\A|/)\.make\.state\Z # (\A|/)\.nse_depinfo\Z # (\A|/)core\Z # (\A|/)tags\Z # (\A|/)TAGS\Z # p 'lib/CLI/Dispatch/Help.pm' 8369 1359486746 0100644 #### End of ApplyPatch data #### #### End of Patch kit [created: Tue Jan 29 14:16:37 2013] #### #### Patch checksum: 94 2987 54794 #### #### Checksum: 112 3687 47145 ####
Hi, it's very interesting to have sub commands. I have never thought of that seriously. However, your current way of implementation looks a bit weird to me. You can do something like this: package X::B; use CLI::Dispatch; use base 'CLI::Dispatch::Command'; sub run { my $self = shift; CLI::Dispatch->run(ref $self); # and put your subcommands under X::B::* } 1; As for the recursive help search, I've often used it to provide a structured manual, so I won't remove it. However, 1) you can provide your own X::Help command, and 2) it may be nice to skip searching if we find a line like "use CLI::Dispatch;" in a (sub) command. Give me some time to consider, or you can send me another patch :) Thanks for an interesting idea! On Tue Jan 29 09:00:00 2013, DJERIUS wrote: Show quoted text
> Hi, > > Thanks for writing CLI::Dispatch. I've run across a small problem
with Show quoted text
> CLI::Dispatch::Help::list_commands. > > I'm attempting to create "sub" commands, as in: > > bin/x b c > > where c is a subcommand for b. This is working quite well, actually. > Below is a layout of the code (hopefully this survives reformatting) > > . > ├── bin > │ └── x > └── lib > ├── X > │ ├── A.pm > │ ├── B > │ │ ├── X > │ │ │ └── C.pm > │ │ └── X.pm > │ └── B.pm > └── X.pm > > > and B.pm is a mix of a command and a top level dispatcher: > > package X::B; > > use CLI::Dispatch; > > use base 'CLI::Dispatch::Command'; > > use X::B::X; > > > sub run { > > X::B::X->run; > > } > > 1; > > > The only problem that I've come across so far is that > CLI::Dispatch::Help::list_commands searches deep into the namespaces, > and finds the sub-commands when I believe it shouldn't, with odd
results: Show quoted text
> > % bin/x > a - x::a > b - x::b > c - [disabled: compile error] > help - show help > x - [disabled: compile error] > > It's finding X::B::X.pm and X::B::X::C.pm when it shouldn't.
X::B::X.pm Show quoted text
> isn't a command at all, and X::B::X::C.pm is b's c command. I
expected Show quoted text
> something like this: > > % bin/x > a - x::a > b - x::b > help - show help > > With help for b's subcommands available when I run > > % bin/x b > c - x b x c > help - show help > > (that does work fine). > > It doesn't look like CLI::Dispatch::Help is performing the deep search > for any real reason; it's not attempting to build a list of sub
commands Show quoted text
> (as %found is keyed off of the basename of the module, not the
relative Show quoted text
> path to the top of the namespace). > > Is the deep search necessary? I realize that what I'm doing isn't
using Show quoted text
> the documented behavior, but the implementations of sub-commands is so > straightforward that it'd be a shame not to be able to use it. > > Thanks, > > Diab > > P.S. I've attached a zip file of the example code.
Subject: Re: [rt.cpan.org #83019] CLI::Dispatch::Help searches too deep
Date: Wed, 30 Jan 2013 14:03:06 -0500
To: bug-CLI-Dispatch [...] rt.cpan.org
From: Diab Jerius <dj [...] head.cfa.harvard.edu>
On Tue, 2013-01-29 at 19:51 -0500, Kenichi Ishigaki via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=83019 > > > Hi, > > it's very interesting to have sub commands. I have never thought of > that seriously. However, your current way of implementation looks a bit > weird to me. You can do something like this: > > package X::B; > > use CLI::Dispatch; > use base 'CLI::Dispatch::Command'; > > sub run { > my $self = shift; > > CLI::Dispatch->run(ref $self); # and put your subcommands under > X::B::* > } > > 1;
I did it the other way so that both the command and subcommand get options, for example x b --b_option c --c_option Because CLI::Dispatch::run grabs global options from the class which invokes it, invoking CLI::Dispatch->run( ref $self ) results in %global = CLI::Dispatch->get_options( CLI::Dispatch->options ); which won't pick up the options for the b command. The extra class is needed to define the options. The class based behavior of CLI::Dispatch limits what can be done when creating sub commands, as it can only handle two levels of options and there's no easy way of passing upper levels. For example, I'd like to be able to do this: x --x_option b --b_option c --c_option To illustrate where I'd like to go with this, I've attached test code for something I'm calling (as a placeholder) CLI::Dispatch::Extended. It does two things: * it supports multiple levels of sub-commands keeping track of the command line options at each level * it provides for reading a configuration file which automatically populates the options for each level. I used Config::Tiny for now; eventually it would be configurable. Command line options override what's in the configuration file. Because CLI::Dispatch is class based I've had to pass options to lower commands via package based caches. I've also had to stick some bookkeeping information in the options hash (similar to what you do with _namespaces). It'd be cleaner if one could instantiate CLI::Dispatch Here's an example of running the newer code (everything needed is in the attached archive): % bin/x a X::A: X = X::A: A = % bin/x --config foo.conf a X::A: X = 1 X::A: A = 1 % bin/x --config foo.conf -X 2 a X::A: X = 2 X::A: A = 1 % bin/x --config foo.conf -X 2 a -A 2 X::A: X = 2 X::A: A = 2 % bin/x b c X::B: X = X::B: B = X::B::X::C: X = X::B::X::C: B = X::B::X::C: C = % bin/x --config foo.conf b c X::B: X = 1 X::B: B = 1 X::B::X::C: X = 1 X::B::X::C: B = 1 X::B::X::C: C = 1 % bin/x --config foo.conf -X 2 b c X::B: X = 2 X::B: B = 1 X::B::X::C: X = 2 X::B::X::C: B = 1 X::B::X::C: C = 1 % bin/x --config foo.conf -X 2 b -B 2 c X::B: X = 2 X::B: B = 2 X::B::X::C: X = 2 X::B::X::C: B = 2 X::B::X::C: C = 1 % bin/x --config foo.conf -X 2 b -B 2 c -C 2 X::B: X = 2 X::B: B = 2 X::B::X::C: X = 2 X::B::X::C: B = 2 X::B::X::C: C = 2
Download cli-dispatch-extended.zip
application/zip 5.9k

Message body not shown because it is not plain text.

Code (with slight cleanups) is up on bitbucket: https://bitbucket.org/djerius/cli-dispatch-extended
Hi. I just uploaded CLI::Dispatch 0.17 on CPAN. Now you can create a CLI::Dispatch object to pass options before dispatching, and ::Help should hide subcommands that are not located in the main command's namespace(s). Hope this helps. On Thu Jan 31 04:03:27 2013, dj@head.cfa.harvard.edu wrote: Show quoted text
> On Tue, 2013-01-29 at 19:51 -0500, Kenichi Ishigaki via RT wrote:
> > <URL: https://rt.cpan.org/Ticket/Display.html?id=83019 > > > > > Hi, > > > > it's very interesting to have sub commands. I have never thought of > > that seriously. However, your current way of implementation looks a
bit Show quoted text
> > weird to me. You can do something like this: > > > > package X::B; > > > > use CLI::Dispatch; > > use base 'CLI::Dispatch::Command'; > > > > sub run { > > my $self = shift; > > > > CLI::Dispatch->run(ref $self); # and put your subcommands under > > X::B::* > > } > > > > 1;
> > > I did it the other way so that both the command and subcommand get > options, for example > > x b --b_option c --c_option > > Because CLI::Dispatch::run grabs global options from the class which > invokes it, invoking CLI::Dispatch->run( ref $self ) results in > > %global = CLI::Dispatch->get_options( CLI::Dispatch->options ); > > which won't pick up the options for the b command. The extra class is > needed to define the options. > > The class based behavior of CLI::Dispatch limits what can be done when > creating sub commands, as it can only handle two levels of options and > there's no easy way of passing upper levels. For example, I'd like
to Show quoted text
> be able to do this: > > x --x_option b --b_option c --c_option > > To illustrate where I'd like to go with this, I've attached test code > for something I'm calling (as a placeholder) CLI::Dispatch::Extended. > It does two things: > > * it supports multiple levels of sub-commands keeping track of
the Show quoted text
> command line options at each level > * it provides for reading a configuration file which
automatically Show quoted text
> populates the options for each level. I used Config::Tiny for > now; eventually it would be configurable. Command line
options Show quoted text
> override what's in the configuration file. > > Because CLI::Dispatch is class based I've had to pass options to lower > commands via package based caches. I've also had to stick some > bookkeeping information in the options hash (similar to what you do
with Show quoted text
> _namespaces). It'd be cleaner if one could instantiate CLI::Dispatch > > Here's an example of running the newer code (everything needed is in
the Show quoted text
> attached archive): > > % bin/x a > X::A: X = > X::A: A = > > % bin/x --config foo.conf a > X::A: X = 1 > X::A: A = 1 > > % bin/x --config foo.conf -X 2 a > X::A: X = 2 > X::A: A = 1 > > % bin/x --config foo.conf -X 2 a -A 2 > X::A: X = 2 > X::A: A = 2 > > % bin/x b c > X::B: X = > X::B: B = > X::B::X::C: X = > X::B::X::C: B = > X::B::X::C: C = > > % bin/x --config foo.conf b c > X::B: X = 1 > X::B: B = 1 > X::B::X::C: X = 1 > X::B::X::C: B = 1 > X::B::X::C: C = 1 > > % bin/x --config foo.conf -X 2 b c > X::B: X = 2 > X::B: B = 1 > X::B::X::C: X = 2 > X::B::X::C: B = 1 > X::B::X::C: C = 1 > > % bin/x --config foo.conf -X 2 b -B 2 c > X::B: X = 2 > X::B: B = 2 > X::B::X::C: X = 2 > X::B::X::C: B = 2 > X::B::X::C: C = 1 > > % bin/x --config foo.conf -X 2 b -B 2 c -C 2 > X::B: X = 2 > X::B: B = 2 > X::B::X::C: X = 2 > X::B::X::C: B = 2 > X::B::X::C: C = 2 >
On Tue Feb 05 09:29:15 2013, ISHIGAKI wrote: Show quoted text
> Hi. > > I just uploaded CLI::Dispatch 0.17 on CPAN. Now you can create a > CLI::Dispatch object to pass options before dispatching, and ::Help > should hide subcommands that are not located in the main command's > namespace(s). > > Hope this helps. >
It does! So far everything works quite nicely. Sorry for the delay in replying; I was only able to get back to this part of my project yesterday. Thanks very much for your help.