#5225 AppGenerator generates incorrect application module name under certain circumstances - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5225 ✓resolved

AppGenerator generates incorrect application module name under certain circumstances

Reported by Fred Wu | July 28th, 2010 @ 04:53 AM

In situations such as running rake rails:update:configs for an existing rails app, the generated files contain application module name Config. This is caused by the destination_root being set to the 'config' folder.

In order to solve this, we need to check to see if an application module name has already been given. If so, use the existing module name, otherwise try figure out the name from the root folder (which is the old behaviour).

Attached is the patch for the fix.

P.S. The patch is against the rails master (as of the time of submitting this ticket).

app_generator_fix.diff 2.7 KB

Comments and changes to this ticket

You flagged this item as spam.
Fred Wu July 28th, 2010 @ 04:58 AM
- Tag changed from “appgenerator, app_name, generators, thor” to “rails 3, rails 3.0 generators, appgenerator, app_name, edge, generators, thor”
You flagged this item as spam.
Fred Wu July 28th, 2010 @ 06:57 AM
- Tag changed from “rails 3, rails 3.0 generators, appgenerator, app_name, edge, generators, thor” to “rails 3, rails 3.0 generators, appgenerator, app_name, edge, generator, generators, patch, thor”
You flagged this item as spam.
Damien MATHIEU July 28th, 2010 @ 08:57 AM
Actually it's already been fixed here : http://github.com/rails/rails/commit/5d3e8ee2bdd2225fe6e610c7e465bb...
You flagged this item as spam.
Fred Wu July 28th, 2010 @ 09:25 AM
Thanks Damien, I overlooked that commit.

Having said that, this patch works a bit differently (I guess it's kinda like a feature request too) -

If we've generated the app and moved its location (i.e. the app folder's name is changed), the app_name would still parse the name from the folder name. This will result in inconsistent application module naming.

My patch seeks for an existing app module name and use that, so it is more accurate.

Any thoughts?
You flagged this item as spam.
Damien MATHIEU July 28th, 2010 @ 10:02 AM
- Assigned user set to “José Valim”
That's a question of philosophy and it should be discussed with the core team.
The last change on app_const_base was by José. I'm assigning this ticket to him.
José Valim July 28th, 2010 @ 10:07 AM
- Importance changed from “” to “Low”
Fred, if you are accessing Rails.application, it means a Rails::Application is defined. Couldn't you then simply do: Rails.application.class.name.sub(/::Application$/, "")? The patch in general looks good.
You flagged this item as spam.
Fred Wu July 28th, 2010 @ 01:29 PM
Hi Jose,

I just did a bit more testing. Rails.application can be access fine within the real world environment, but in testing, it does not work because it has a 'hacked-up' Rails app called TestApp (defined in test/abstract_unit). This is why in the AppGenerator Rails.appilcation.config.root is used instead of Rails.root.

Do you guys have any suggestions?

If the original AppGenerator changes are acceptable, I will submit a new patch with updated test.
José Valim July 28th, 2010 @ 01:34 PM
The idea of the original patch is correct, but I would prefer if it could use Rails.application.class.name. About the tests, you can stub Rails.application to return an instance of a fake class with the name you want to test! Thanks!
You flagged this item as spam.
Fred Wu July 28th, 2010 @ 01:58 PM
Thanks Jose. (Apparently I'm too unwell to remember stubing.)

Attaches is the updated patch. :)
- app_generator_fix.diff 3 KB
Repository August 2nd, 2010 @ 03:55 PM
- State changed from “new” to “resolved”
(from [cdad483dff4fef1b640dc3c750719c325b252f89]) Improved how AppGenerator generates the application name. It now detects the current app name whenever possible. This means that renaming the residing directory will not effect the app name generated by AppGenerator.

[#5225 state:resolved]

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/cdad483dff4fef1b640dc3c750719c...