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.

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

post-argument-parsing hook #81

Closed
smackesey opened this issue Oct 16, 2014 · 11 comments
Closed

post-argument-parsing hook #81

smackesey opened this issue Oct 16, 2014 · 11 comments

Comments

@smackesey
Copy link

Is there any way to do some processing (add to options object, set instance variables and so on) after all options have been parsed but before any command has been run? If not, this would be a useful feature (I'd be willing to implement it in a PR)-- if it exists, then I think it should be mentioned in the README.

@ggilder
Copy link
Collaborator

ggilder commented Oct 20, 2014

I don't think what you describe exists today. However, I'm having a hard time imagining a good use case for such a feature. Could you maybe give a short example of how such a feature would be used?

@smackesey
Copy link
Author

@ggilder Sure. The use I have in mind is for global options. As far as I'm aware, there is no way to invoke the #default method of the option struct before you're inside an action block. This requires you to repeat code if you want to set defaults on global options. The same goes for any processing of the global options that'd you like to do (e.g. loading some data from a database depending on an option value)-- if a value is passed, you can do this in block of the global_option method, but not if you want to do processing on a default value.

@ggilder
Copy link
Collaborator

ggilder commented Oct 26, 2014

Hmm, maybe I'm mistaken but setting defaults seems pretty straightforward already. I'd probably use something like the following:

GLOBAL_DEFAULTS = { foo: 'bar' }
global_option('--foo VALUE') { |val| GLOBAL_DEFAULTS[:foo] = val }

Similarly with command options, the default method just takes a hash so it's easy enough to extract that:

COMMAND_DEFAULTS = { prefix: 'foo' }
command :bar do |c|
  c.option '--prefix STRING', String, 'Adds a prefix to bar'
  c.action do |args, options|
    options.default COMMAND_DEFAULTS
  end
end
# repeat with other commands...

As far as post-processing the options, the examples you give seem pretty domain-specific, so my instinct would be to delegate that to your own classes outside of the commander DSL:

class AppSetup
  def self.setup(options)
    # your own logic to do db queries, set up objects, etc.
  end
end

command :bar do |c|
  c.action do |args, options|
    # delegate post-processing, setup, etc. to your domain objects
    AppSetup.setup(options)
  end
end

Does that address the use cases you're thinking of? If not, maybe a short code sample of what you're imagining would be helpful for discussion.

@akostadinov
Copy link

What @smackesey is seems to be describing and what I see useful is to allow initialization based on global options. For example a program doing DB query may want to open a DB connection regardless of command in use. And without need for all commands to call same initialization method.

def run
      program :name, 'MongoQuery'
      default_command :ui
      command :ui do |c|
        ...
      end

      pre_command do |c|
        c.action do |args, global_options|
          # put common initialization here
        end
      end
end 
...

@ggilder
Copy link
Collaborator

ggilder commented Feb 16, 2015

@akostadinov practically speaking, couldn't you just run that initialization code before the commander stuff? e.g.:

def run
  # put common initialization here

  program :name, 'MongoQuery'
  default_command :ui
  # etc...
end

I don't really see what advantage you'd get by putting that in a commander hook.

@akostadinov
Copy link

@ggilder , in your example I don't have access to parsed global_options. That's the issue. e.g. I don't have access to database credentials before parameters are parsed.

@ggilder
Copy link
Collaborator

ggilder commented Feb 17, 2015

Thanks, I think I see the use case now. I think this would be a relatively straightforward change to Commander::Runner#parse_global_options. If you're interested in putting together a pull request I can help with any questions you have.

akostadinov added a commit to akostadinov/commander that referenced this issue Feb 17, 2015
@akostadinov
Copy link

Please comment on pull #96

@akostadinov
Copy link

On the other hand this whole feature might be stupid after all. If I want to get a database, then I can have a method to get me the DB connection object (in contrast with setting up an @db variable within the after_global_options hook). When I call that #db method it will either return @db if it exists or try creating a new connection.
I'm not sure there are any valid use cases.
Another thing is that if one does setup in the after_global_options hook, then why do it if command is help for example?

Everybody interested, please comment.

@ggilder
Copy link
Collaborator

ggilder commented Feb 18, 2015

I guess my assumption is that on something like help the user is not providing the necessary global options that would cause the expensive connection to get set up.

However, I personally agree with the skepticism — for something that needs a complex level of setup I would probably prefer to have that managed by my own module, using Commander as the translation between CLI and this module.

@ggilder
Copy link
Collaborator

ggilder commented Apr 1, 2015

This issue was moved to commander-rb/commander#5

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

No branches or pull requests

3 participants