-
Notifications
You must be signed in to change notification settings - Fork 615
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
[Fixes #260] Make space trimming consistent for all task arguments #259
Conversation
2ab7603
to
15f9169
Compare
def test_can_handle_spaces_between_args | ||
name, args = @app.parse_task_string("name[one, two,\tthree , \tfour]") | ||
def test_can_handle_spaces_between_all_args | ||
name, args = @app.parse_task_string("name[ one , two ,\tthree , \tfour ]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the existing test and add a new test as this is technically a new feature. Also, make sure it doesn't strip out whitespaces in a string literal:
rake name[one,two,three] # => should be interpreted as `rake name[one,two,three]`
rake name[ one , two, three] # => should be interpreted as `rake name[one,two,three]`
rake name["one two",three] # => should be interpreted as `rake name["one two",three]`
rake name[ "one two", three] # => should be interpreted as `rake name["one two",three]`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback.
Added original test back.
Regarding your second comment, i think there's already a test case below testing that white-spaces embedded in arguments are not stripped out. Let me know if that covers it or not.
Added new commit, let me know if you prefer amend.
0e2cd0f
to
7175910
Compare
Thanks! |
Fixes #260
Given a task that just inspect task arguments...
Current rake behavior
With this PR changes