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

Allow spaces between arguments in Rake tasks #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bronzdoc
Copy link

This pull request allows input like this rake my_task[arg1, arg2] to be parsed correctly
instead of showing the error Don't know how to build task 'mytask[arg1, ' as described
in https://github.com//issues/85

@hsbt
Copy link
Member

hsbt commented Jan 7, 2016

Please add testcase.

@bronzdoc
Copy link
Author

bronzdoc commented Jan 7, 2016

@hsbt there's already a test for this test_can_handle_spaces_between_args in test/test_rake_task_argument_parsing.rb

@tarasmatsyk
Copy link

hi everyone,
what is the state of this pull request? I see the test in 10.4.2 version but I cannot find the code there.

@hsbt
Copy link
Member

hsbt commented May 21, 2016

Why we faced this defects as #85 with test_can_handle_spaces_between_args ?

I hope to add another test-case.

@@ -84,6 +84,11 @@ def init(app_name='rake')
standard_exception_handling do
@name = app_name
args = handle_options
if args.size > 1 && args.last =~ /.*\]/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .* is not needed in the regular expression.

@@ -84,6 +84,11 @@ def init(app_name='rake')
standard_exception_handling do
@name = app_name
args = handle_options
if args.size > 1 && args.last =~ /.*\]/
clean_args = ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could just be

if args.size > 1 && args.last =~ /.*\]/
  args = [args.join]
end

right?

Also if the only thing that's stopping this from going out is more tests, I'd be happy to write them.

@aycabta
Copy link
Member

aycabta commented Sep 10, 2017

I think that this is just shell matter and you should use quoted string literal. In the Bash example, please check Bash Reference Manual's several points, Quoting and Word Splitting.

The quoted string literal and word splitting of shell is so complex because of histrical background and Rake is sometimes called by complex shell scripts, such as inside CI, Dockerfile and build scripts. I think this Pull Request is precarious.

For another thing, task name can take any string:

task :'task]' do
  puts 'This is task]'
end

The task name what is ended of "]" is within the realm of possibility, for example, some kind of model number because Ruby is general-purpos programming language.

@rickhull
Copy link
Contributor

rickhull commented Nov 6, 2017

@aycabta's concerns make sense to me. I think @drbrain's comment here is the right approach. Shell strings, quoting, splitting, etc are gnarly, and I suspect it's best to bite the bullet and be explicit by quoting embedded spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants