Subcommand fixes (Close #241) #263

Merged
merged 5 commits into from Oct 18, 2012

2 participants

@ahawkins

This PR's fixes some edge cases around subcommands (#241). You can now correctly invoke subcommands with a default task with 0, 1, or many args.

@sferik sferik and 1 other commented on an outdated diff Oct 17, 2012
Gemfile
-platforms :mri_19 do
- gem 'ruby-debug19'
-end
+# platforms :mri_19 do
+# gem 'ruby-debug19'
+# end
@sferik
Erikhuda member
sferik added a note Oct 17, 2012

Did you intend to commit these changes? Were these lines causing problems for you? We should probably just specify the debugger gem instead.

@sferik
Erikhuda member
sferik added a note Oct 17, 2012

Apparently, you're a step ahead of me. Still this diff doesn't belong in this pull request.

No I did not mean to commit to those changes. What's the easiest way to remove them?

@sferik
Erikhuda member
sferik added a note Oct 17, 2012

Undo the changes. Then:

git add Gemfile
git commit --amend
git push -f origin subcommand-fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ahawkins

@sferik that should be much cleaner now :)

@sferik sferik commented on an outdated diff Oct 17, 2012
lib/thor.rb
@@ -255,8 +256,33 @@ def check_unknown_options?(config) #:nodoc:
# The method responsible for dispatching given the args.
def dispatch(meth, given_args, given_opts, config) #:nodoc:
- meth ||= retrieve_task_name(given_args)
- task = all_tasks[normalize_task_name(meth)]
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
@sferik
Erikhuda member
sferik added a note Oct 17, 2012

Looks okay, except for this part. Why not pass this in the config hash instead of given_args?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eventualbuddha eventualbuddha commented on an outdated diff Oct 17, 2012
@@ -255,8 +256,33 @@ def check_unknown_options?(config) #:nodoc:
# The method responsible for dispatching given the args.
def dispatch(meth, given_args, given_opts, config) #:nodoc:
- meth ||= retrieve_task_name(given_args)
- task = all_tasks[normalize_task_name(meth)]
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
+ via_subcommand = !!given_opts.delete('--invoked-via-subcommand')
+ else
+ via_sucommand = false
+ end
+
+ # This is an edge case. If this is dispathced from a subcommand

s/dispathced/dispatched/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eventualbuddha eventualbuddha commented on an outdated diff Oct 17, 2012
@@ -255,8 +256,33 @@ def check_unknown_options?(config) #:nodoc:
# The method responsible for dispatching given the args.
def dispatch(meth, given_args, given_opts, config) #:nodoc:
- meth ||= retrieve_task_name(given_args)
- task = all_tasks[normalize_task_name(meth)]
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
+ via_subcommand = !!given_opts.delete('--invoked-via-subcommand')
+ else
+ via_sucommand = false
+ end
+
+ # This is an edge case. If this is dispathced from a subcommand
+ # and there is may be one argument. This causes a problem when

"there is may be"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eventualbuddha eventualbuddha commented on an outdated diff Oct 17, 2012
@@ -255,8 +256,33 @@ def check_unknown_options?(config) #:nodoc:
# The method responsible for dispatching given the args.
def dispatch(meth, given_args, given_opts, config) #:nodoc:
- meth ||= retrieve_task_name(given_args)
- task = all_tasks[normalize_task_name(meth)]
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
+ via_subcommand = !!given_opts.delete('--invoked-via-subcommand')
+ else
+ via_sucommand = false
+ end
+
+ # This is an edge case. If this is dispathced from a subcommand
+ # and there is may be one argument. This causes a problem when
+ # the given argument is an argument for the default task. This situation
+ # can only occur when there one argument and a real default task

s/there one/there is one/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eventualbuddha eventualbuddha commented on an outdated diff Oct 17, 2012
- meth ||= retrieve_task_name(given_args)
- task = all_tasks[normalize_task_name(meth)]
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
+ via_subcommand = !!given_opts.delete('--invoked-via-subcommand')
+ else
+ via_sucommand = false
+ end
+
+ # This is an edge case. If this is dispathced from a subcommand
+ # and there is may be one argument. This causes a problem when
+ # the given argument is an argument for the default task. This situation
+ # can only occur when there one argument and a real default task
+ # is present. Thor use "help" by default so we skip that case.

s/use/uses/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eventualbuddha eventualbuddha commented on an outdated diff Oct 17, 2012
- task = all_tasks[normalize_task_name(meth)]
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
+ via_subcommand = !!given_opts.delete('--invoked-via-subcommand')
+ else
+ via_sucommand = false
+ end
+
+ # This is an edge case. If this is dispathced from a subcommand
+ # and there is may be one argument. This causes a problem when
+ # the given argument is an argument for the default task. This situation
+ # can only occur when there one argument and a real default task
+ # is present. Thor use "help" by default so we skip that case.
+ # Note the call to retreive_task_name. It's called with

s/retreive_task_name/retrieve_task_name/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eventualbuddha eventualbuddha commented on an outdated diff Oct 17, 2012
+ if given_opts
+ #FIXME: is there a better way to pass information from the
+ # subcommand invocation? It has to go through invoke which
+ # only accepts *args
+ via_subcommand = !!given_opts.delete('--invoked-via-subcommand')
+ else
+ via_sucommand = false
+ end
+
+ # This is an edge case. If this is dispathced from a subcommand
+ # and there is may be one argument. This causes a problem when
+ # the given argument is an argument for the default task. This situation
+ # can only occur when there one argument and a real default task
+ # is present. Thor use "help" by default so we skip that case.
+ # Note the call to retreive_task_name. It's called with
+ # given_args.dup since that method calls args.shift. First Attempt

s/Attempt/attempt/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
twinturbo added some commits Oct 18, 2012
@ahawkins

@sferik @eventualbuddha addressed all your comments in two new comments. Rework comment for @eventualbuddha and pass flag via subcommand for @sferik.

@wycats wycats merged commit 976de90 into erikhuda:master Oct 18, 2012

1 check failed

Details default The Travis build failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment