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

Fix argument passing to rake routes #23612

Merged
merged 1 commit into from
Feb 12, 2016

Conversation

vipulnsward
Copy link
Member

  • rake tasks need to be prepended with -- , so as to make sure they are not parsed by rake.
  • Fixes routes task to take a note of this and parse arguments accordingly
  • Fixed related documentation and usage all around

Fixes #23561

@prathamesh-sonpatki
Copy link
Member

r? @kaspth

@kaspth
Copy link
Contributor

kaspth commented Feb 11, 2016

Users shouldn't have to do this. We should make --controller work directly.

@vipulnsward
Copy link
Member Author

@kaspth routes has been a rake task before. To still use it that way, we will need to let it remain a rake task, to be backward compatible.
As mentioned rake tasks need the -- switch for custom arguments. So to support this in existing task, we need to use it with this switch.

Other option is we drop the rake task support and convert this to a rails command.

@kaspth
Copy link
Contributor

kaspth commented Feb 11, 2016

-g and -c works without --, right?

@dhh how does having to pass -- for long style arguments look to you? E.g. rails routes -- --controller carts

@dhh
Copy link
Member

dhh commented Feb 11, 2016

You have to do -- twice? That doesn't work for me at all. May even be worse than passing ENVs like Rake did.

@dhh
Copy link
Member

dhh commented Feb 11, 2016

To be more productive. If we can't do long-version nicely, maybe just don't do long version at all? -g and -c seem fine to me.

@kaspth
Copy link
Contributor

kaspth commented Feb 11, 2016

Yeah, I was thinking that too. Would be a better compromise than automatically trying to insert -- for instance. @vipulnsward can you update this to remove the long style args?

@vipulnsward vipulnsward force-pushed the 23561-fix-routes-args branch 2 times, most recently from 7c3d55f to b7c9272 Compare February 12, 2016 07:14
@vipulnsward
Copy link
Member Author

Ok, updated to remove long arguments. Short args work as expected.

@@ -19,11 +19,11 @@ task routes: :environment do

OptionParser.new do |opts|
opts.banner = "Usage: rails routes [options]"
opts.on("-c", "--controller [CONTROLLER]") do |controller|
opts.on("-cCONTROLLER") do |controller|
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we have a space between -c and CONTROLLER?

- Fixed related documentation and usage all around

Fixes rails#23561
@vipulnsward
Copy link
Member Author

Done.

kaspth added a commit that referenced this pull request Feb 12, 2016
@kaspth kaspth merged commit f709682 into rails:master Feb 12, 2016
@kaspth
Copy link
Contributor

kaspth commented Feb 12, 2016

Sweet!

@vipulnsward
Copy link
Member Author

🍠

@vipulnsward vipulnsward deleted the 23561-fix-routes-args branch February 22, 2016 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants