make the current command chain accessible in command #348

Merged
merged 2 commits into from Mar 22, 2014

5 participants

@maxmeyer

hi there,

for a project I need information about the commands which has been used to invoke the executable. This PR makes the commands accessible within a thor task via current_command_chain as a sorted array of symbols.

Would you mind to review the PR and give feedback to the current implementation?

Thank you very much!

Cheers,
MaxMeyer

@sferik
Erikhuda member

If you still want this patch, can you please pull in the latest master and rebase this branch?

@maxmeyer

Rebased... Ok?

@sferik sferik commented on the diff Dec 16, 2013
lib/thor/invocation.rb
@@ -25,6 +25,11 @@ def initialize(args = [], options = {}, config = {}, &block) #:nodoc:
super
end
+ # Make the current command chain accessible with in a Thor-(sub)command
+ def current_command_chain
+ @_invocations.values.flatten.map(&:to_sym)
@sferik
Erikhuda member
sferik added a line comment Dec 16, 2013

If this is going to be public, the instance variable should not start with an underscore. I’m trying to think if there was a good reason why this needs to be private. /cc @josevalim

@josevalim
josevalim added a line comment Dec 23, 2013

We don't want our instance variables to clash with the user ones. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sferik
Erikhuda member

It looks like this patch causes a spec to fail on Ruby 1.8.7 because it depends on hash ordering. Perhaps that’s why this variable was private to begin with?

@maxmeyer

I can change the test to check only if the commands are included...

@sferik
Erikhuda member

Yes, please.

@coveralls

Coverage Status

Coverage increased (+0.18%) when pulling 4929c95 on maxmeyer:feature/current_command into 3dd0e8f on erikhuda:master.

@sferik
Erikhuda member

Did you see my note on the name of the @_invocations instance variable?

@maxmeyer

I saw it, that, but I thought you wanted @josevalim to comment on that?!

@josevalim

This PR makes the commands accessible within a thor task via current_command_chain as a sorted array of symbols.

I don't see anything in the code that makes it sorted. Is it an issue in the description or in the code? :)

@maxmeyer

It is an issue in the description: The sort criteria is the order of the commands on commandline (>= ruby 1.9.x)

my.rb cmd1 cmd2
pp @_invocations
#[ :cmd1, :cmd2]
@maxmeyer

@josevalim Should I change anything else?

@dg-ratiodata

Anything missing?

@sferik sferik merged commit d6e3037 into erikhuda:master Mar 22, 2014

1 check passed

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