rails destroy generator for controller accepts all names #7827

Closed
judearasu opened this Issue Oct 3, 2012 · 15 comments

Projects

None yet

7 participants

@judearasu
Contributor

Currently using rails 3.2.8 version. I found this bug when i tried to delete a controller sass.

Mistakenly i typed
rails d controller ssss instead of rails d controller sass

Console is showing the messsge as ssss.js.coffee , ssss_controller.rb removed

As i dont have controller ssss, then why it has to accept the wrong name.

@steveklabnik
Member

This is not a bug.

@steveklabnik steveklabnik reopened this Oct 3, 2012
@steveklabnik
Member

... that said, maybe someone else might think it is.

@senny
Member
senny commented Oct 3, 2012

It feels odd that you get this misleading output:

my-project :: (master*) » rails d controller idonotexist
      remove  app/controllers/idonotexist_controller.rb
      invoke  erb
      remove    app/views/idonotexist
      invoke  test_unit
      remove    test/functional/idonotexist_controller_test.rb
      invoke  helper
      remove    app/helpers/idonotexist_helper.rb
      invoke    test_unit
      remove      test/unit/helpers/idonotexist_helper_test.rb
      invoke  assets
      invoke    js
      remove      app/assets/javascripts/idonotexist.js
      invoke    scss
      remove      app/assets/stylesheets/idonotexist.css.scss

I think when you got a hard-to-see typo like the one above this can lead to a few minutes of frustration. @steveklabnik why do you think we should not fix this? Is it hard to decide wether the controller actually exists or are there other reasons not to touch this?

@steveklabnik
Member

Well, for one, I'm not sure of what all the implications are. What if the controller doesn't exist, but the helpers do? The tests? The views?

I see rake tasks as macros. After running this task, the idonotexist controller is deleted. Doesn't matter to me if it existed or not.

I think there's just too many edge cases to properly make this 'work.'

@judearasu
Contributor

Thanx @senny for your explanation this issue more deeply

@frodsan
Contributor
frodsan commented Oct 3, 2012

The same happens with rails d model idonotexist:

$ rails d model idonotexist
      invoke  active_record
      remove    migration.rb
      remove    app/models/idonotexist.rb
      invoke    test_unit
      remove      test/unit/idonotexist_test.rb
      remove      test/fixtures/idonotexists.yml

I agree with Steve. I think this is not a bug.

@judearasu
Contributor

@frodsan This is should be treated as a bug.We need to find out the possible way to fix it.

@senny
Member
senny commented Oct 4, 2012

I think either opinion is fine. I see how @steveklabnik argues that it is just a handful of scripts that you can run but you need to know what is happening. I think we should get one more opinion from a rails committer and then close it or attach a PR.

@rafaelfranca what do you think?

@vijaydev
Member
vijaydev commented Oct 5, 2012

I side on not considering this a bug.

@judearasu judearasu closed this Oct 5, 2012
@judearasu judearasu reopened this Oct 5, 2012
@judearasu
Contributor

@vijaydev Thanks

@steveklabnik
Member

Okay, so we don't consider this a bug. This doesn't mean that it can't go into Rails, but if it's a feature request, we don't keep the issue open. Please submit a PR to change this behavior, or ask for feedback on rails-core. Thanks.

@guilleiguaran
Member

@judearasu just to clarify: if this is a bug is a bug in Thor, no in Rails. Rails is just using actions provided by Thor for generators.

@guilleiguaran
Member

This can be seen like a % rm -fr unexistingfile, if the file doesn't exists it won't raise an error

@rafaelfranca
Member

I agree that this is not a Rails bug as @guilleiguaran pointed.

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