davetron5000 / gli

Make awesome command-line applications the easy way
http://davetron5000.github.io/gli
Apache License 2.0
1.26k stars 102 forks source link

Sub-sub-commands can not have a switch with the same name in gli-2.0.0-rc2. #96

Closed bdiz closed 11 years ago

bdiz commented 12 years ago

This code raises exception.

        command :admin do |c|
            c.command :add do |c_add|
                c_add.flag :milestone
            end
            c.command :delete do |c_delete|
                c_delete.flag :milestone
            end
            c.command :misc do |c_misc|
                c_misc.switch :verbose
            end
        end

gli/dsl.rb:171:in verify_unused_in_option': milestone has already been specified as a flag in the command admin (ArgumentError) from gli/dsl.rb:164:inblock in verify_unused' from gli/dsl.rb:163:in each' from gli/dsl.rb:163:inverify_unused' from gli/dsl.rb:65:in flag' from gli/command_support.rb:42:inflag' from gli/command_support.rb:40:in flag' from options.rb:97:inblock (2 levels) in define_admin_commands'

davetron5000 commented 12 years ago

Yeah this was sort-of a design decision. I thought supporting it might be confusing but NOT supporting it is, too. I'll keep this open but might need to wait for 2.1 as it's a big change.

Dave

(Sent from my iPhone [I CAN spell])

On Jun 27, 2012, at 5:14 PM, bdizreply@reply.github.com wrote:

This code raises exception.

       command :admin do |c|
           c.command :add do |c_add|
               c_add.flag :milestone
           end
           c.command :delete do |c_delete|
               c_delete.flag :milestone
           end
           c.command :misc do |c_misc|
               c_misc.switch :verbose
           end
       end

gli/dsl.rb:171:in verify_unused_in_option': milestone has already been specified as a flag in the command admin (ArgumentError) from gli/dsl.rb:164:inblock in verify_unused' from gli/dsl.rb:163:in each' from gli/dsl.rb:163:inverify_unused' from gli/dsl.rb:65:in flag' from gli/command_support.rb:42:inflag' from gli/command_support.rb:40:in flag' from options.rb:97:inblock (2 levels) in define_admin_commands'


Reply to this email directly or view it on GitHub: https://github.com/davetron5000/gli/issues/96

aussielunix commented 12 years ago

I hit this issue around 12hrs ago too and was just looking through tickets before I raised one. For a workaround I was able to just move the switches/flags up to the parent command. It's not perfect but it gets me past this for now. I just wrote this code without testing so it may not run verbatim

command :admin do |c|
  c.desc "name of project"
  c.flag :name

  c.desc 'milestone'
  c.flag :milestone

  c.desc 'be verbose'
  c.switch :verbose

  c.command :add do |c_add|
    c_add.action do |global_options,options,args|
      #cmd = foo::bar(options[:name], options[:milestone])
      #puts 'verbose things' if options[:verbose]
    end
  end

  c.command :delete do |c_delete|
    c_delete.action do |global_options,options,args|
      #cmd = foo::baz(options[:name], options[:milestone])
      #puts 'verbose things' if options[:verbose]
    end
  end
end  
bdiz commented 12 years ago

Only works if all of your sub-commands sub-sub-commands support the flag.

-----Original Message----- From: Mick Pollard [mailto:reply@reply.github.com] Sent: Thursday, June 28, 2012 2:19 AM To: Delsol, Ben Subject: Re: [gli] Sub-sub-commands can not have a switch with the same name in gli-2.0.0-rc2. (#96)

I hit this issue around 12hrs ago too and was just looking through tickets before I raised one. For a workaround I was able to just move the switches/flags up to the parent command. It's not perfect but it gets me past this for now. I just wrote this code without testing so it may not run verbatim

command :admin do |c|
  c.desc "name of project"
  c.flag :name

  c.desc 'milestone'
  c.flag :milestone

  c.desc 'be verbose'
  c.switch :verbose

  c.command :add do |c_add|
    c_add.action do |global_options,options,args|
      #cmd = foo::bar(options[:name], options[:milestone])
      #puts 'verbose things' if options[:verbose]
    end
  end

  c.command :delete do |c_delete|
    c_delete.action do |global_options,options,args|
      #cmd = foo::baz(options[:name], options[:milestone])
      #puts 'verbose things' if options[:verbose]
    end
  end
end

Reply to this email directly or view it on GitHub: https://github.com/davetron5000/gli/issues/96#issuecomment-6625124

aussielunix commented 12 years ago
Only works if all of your sub-commands sub-sub-commands support the flag.

Yeh and i just hit this too.

+1 for having this support added.

bdiz commented 12 years ago

What's the ETA on version 2.1 which will have this feature? The more I develop my CLI the more I'm realizing that this feature is fundamental to building a CLI which is like git. My workarounds in the features absence are making my CLI look very ungit-ish.

davetron5000 commented 12 years ago

Just spent some time looking into this. I didn't get very far. I think the tricky thing is that , if we truly support each command and subcommand having its own "option-space", the arguments to the action block get a little complex.

For example:

command :top do |top|
  top.flag :f

  command :sub1 do |sub1|
    sub1.flag :f

    command :sub1sub1 do |sub1sub1|

      sub1sub1.flag :f

      sub1sub1.action do |global_options,options,args|
      end
    end

    sub1.action do |global_options,options,args|
    end
  end

  top.action do |global_options,options,args|
  end
end

If I invoke my_app top -f top_arg sub1 -f sub1_arg sub1sub -f sub1sub1_arg then what would be in options when the action block gets called?

I'm not sure of a) the correct answer or b) the backwards-compatible answer.

One option would be that options[:f] would return something like {'top' => 'top_arg', 'top sub1' => 'sub1_arg', 'top sub1 sub1sub1' => 'sub1sub1_arg'}. Yech. Any other ideas?

bdiz commented 12 years ago

Thanks for looking into this.

Under most circumstances, the -f value a command would be concerned with is its own. I’d suggest for options[:f] to be different depending on the command action being yielded to; the value for the flag of the current command.

However, it would be nice to get access to other commands -f values. Maybe something like options[:command_name][:f] would be a convenient way.

I don’t think the above ruins backward compatibility, but I’m not sure. The downside to the above is that a sub-command could not have a flag that is named the same as another command, since the keys would clash in options.

From: David Copeland [mailto:notifications@github.com] Sent: Thursday, September 13, 2012 11:47 AM To: davetron5000/gli Cc: Delsol, Ben Subject: Re: [gli] Sub-sub-commands can not have a switch with the same name in gli-2.0.0-rc2. (#96)

Just spent some time looking into this. I didn't get very far. I think the tricky thing is that , if we truly support each command and subcommand having its own "option-space", the arguments to the action block get a little complex.

For example:

command :top do |top|

top.flag :f

command :sub1 do |sub1|

sub1.flag :f

command :sub1sub1 do |sub1sub1|

  sub1sub1.flag :f

  sub1sub1.action do |global_options,options,args|

  end

end

sub1.action do |global_options,options,args|

end

end

top.action do |global_options,options,args|

end

end

If I invoke my_app top -f top_arg sub1 -f sub1_arg sub1sub -f sub1sub1_arg then what would be in options when the action block gets called?

I'm not sure of a) the correct answer or b) the backwards-compatible answer.

One option would be that options[:f] would return something like {'top' => 'top_arg', 'top sub1' => 'sub1_arg', 'top sub1 sub1sub1' => 'sub1sub1_arg'}. Yech. Any other ideas?

— Reply to this email directly or view it on GitHubhttps://github.com/davetron5000/gli/issues/96#issuecomment-8538687.

klaut commented 11 years ago

I just wanted to say that I hit this problem too. I have a series of subcommands that I want them to have the switch --rm available, but not all of them should support it. And having the switch defined on the command level feels a bit awkward.

jasons2645 commented 11 years ago

I've also run into the same issue bdiz has:

"The more I develop my CLI the more I'm realizing that this feature is fundamental to building a CLI which is like git. My workarounds in the features absence are making my CLI look very ungit-ish."

What is the ETA for this fix?

davetron5000 commented 11 years ago

All on this thread, can you try your apps with 2.6.0.rc1?

I've totally reworked how subcommands are implemented, and it should fix this issue, allowing each subcommand to have its own set of flags and switched:

s.add_runtime_dependency('gli','2.6.0.rc1')
davetron5000 commented 11 years ago

Promoted to 2.6.0 that has the fix for this. Wow did it take a long ass time, but hopefully it works better now.

kale commented 10 years ago

Dave, am I wrong, or should this work as of 2.6?

command :reports do |c|
  c.command :sent do |s|
    s.flag :limit
    s.action do |global,options,args|
      not_implemented
    end
  end

  c.command :opened do |s|
    s.flag :limit
    s.action do |global,options,args|
      not_implemented
    end
  end
end

I get a -

`verify_unused_in_option': limit has already been specified as a flag in the command reports (ArgumentError)
davetron5000 commented 10 years ago

Did you set subcommand_option_handling :normal in your app file somewhere? For backwards compatibility, GLI defaults to the original (arguably broken) way of handling this, and would result in the error you are seeing.

davetron5000 commented 10 years ago

Sorry, to expand, a newly scaffolded app should have subcommand_option_handling :normal set so your app operates the new way, but if you had an older pre-2.6 app or didn't scaffold, the value of subcommand_option_handling will be :legacy, which would result in the error you are seeing given that code.

kale commented 10 years ago

Ah, yeah that was the issue (it was pre-2.6)... thanks! I guess I should of looked at the release notes, but maybe that should be added to the overview page?

davetron5000 commented 10 years ago

Put a scare warning on https://github.com/davetron5000/gli/wiki/Subcommands which is the most likely place I can think of that someone would look for docs. Is there any other place you were looking where it would be handy?

kale commented 10 years ago

Awesome, thanks! I think that should be good, but maybe add a link to the subcommand page in the readme? As a new user, I would want to see that page to get a feel of how commands/subcommands work.

kowal commented 9 years ago

Hi @davetron5000. If this issue is fixed then maybe https://github.com/davetron5000/gli/wiki/Subcommands#flags-and-switches section could be updated as well, what do you think?

davetron5000 commented 9 years ago

It's been a long time since this issue was opened—do you want to take a stab at updating the wiki?

kowal commented 9 years ago

@davetron5000 can you please check if something like this would be fine : https://github.com/kowal/gli/wiki/Subcommands-updated.

Side question: How do we access parent-command options? I found that options[GLI::Command::PARENT] works but this isn't public API, right?

davetron5000 commented 9 years ago

That seems ok—are you proposing changing the entire Subcommands page to that? If not, go ahead and edit the GLI wiki directly. I can look at the diff like that and we can tweak it as needed.

GLI::Command::PARENT is public—it's documented in the API docs, but probably not well documented elsewhere.

kowal commented 9 years ago

@davetron5000 I've updated Flags and Switches section. Also added new section Accessing parent command options.

davetron5000 commented 9 years ago

Awesome, thanks! I tweaked it a bit, but this is much better than before

kowal commented 9 years ago

Cool, thanks!