Allow magic code mixing for application controller

#1

Engines’ magic code mixing is mega-cool, but unfortunately it doesn’t
apply to the application controller because of the non-standard file-
naming application.rb, which misses the “_controller” bit and doesn’t
match the regular expression in dependencies.rb.

Is there a reason not to allow this feature for the application
controller? I’d like to move quite some stuff from an application
controller that I’m splitting up into engines … and there’s just no
more comprehensible place to use than plugin_name/app/controllers/
application.rb. :slight_smile:

So, can I apply the following?

I don’t think there’s any potential harm resulting from this.

Thanks!

Index: /engines/lib/engines/rails_extensions/dependencies.rb

— /engines/lib/engines/rails_extensions/dependencies.rb (revision
668)
+++ /engines/lib/engines/rails_extensions/dependencies.rb (working
copy)
@@ -102,7 +102,7 @@
# if we recognise this type
# (this regexp splits out the module/filename from any
instances of app/#{type}, so that
# modules are still respected.)

  •  if file_name =~ /^(.*app\/#{file_type}s\/)?(.*_#{file_type})

(.rb)?$/

  •  if file_name =~ /^(.*app\/#{file_type}s\/)?(.*_#{file_type}|

application)(.rb)?$/
base_name = $2
# … go through the plugins from first started to last, so
that
# code with a high precedence (started later) will override
lower precedence

–
sven fuchs removed_email_address@domain.invalid
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

#2

On Feb 24, 2008, at 8:57 AM, Sven F. wrote:

Engines’ magic code mixing is mega-cool, but unfortunately it doesn’t
apply to the application controller because of the non-standard file-
naming application.rb, which misses the “_controller” bit and doesn’t
match the regular expression in dependencies.rb.

Is there a reason not to allow this feature for the application
controller? I’d like to move quite some stuff from an application
controller that I’m splitting up into engines … and there’s just no
more comprehensible place to use than plugin_name/app/controllers/
application.rb. :slight_smile:
+1 but…

…this could be a major source of conflicts between engines and
should probably be used only when nothing else works. For example
using a subclass of ApplicationController that all your engine
Controllers then subclass would avoid many such conflicts. See what I
mean?

So, can I apply the following?

I don’t think there’s any potential harm resulting from this.
The patch looks harmless enough to me (with the above caveat).

Cheers,
Pascal.

http://blog.nanorails.com

#3

Thanks for the quick answer, Pascal!

Am 24.02.2008 um 21:16 schrieb Pascal:

+1 but…

cool

…this could be a major source of conflicts between engines and
should probably be used only when nothing else works. For example
using a subclass of ApplicationController that all your engine
Controllers then subclass would avoid many such conflicts. See what I
mean?

In general I totally agree.

In my usecase I can safely assume that all engines are designed to fit
together though and readability-wise it really makes most sense to use
ApplicationController here.

The patch looks harmless enough to me (with the above caveat).

Great :slight_smile:

–
sven fuchs removed_email_address@domain.invalid
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

#4

Hi James,

thanks for your insightful response :slight_smile:

Actually I have thought about it and will most probably adopt this
approach (“explicitly opt in”) as things become more visible during
refactoring. Right now I have two acts_as_coffeemaking_controller-type
macros which are included in several controllers. And for these I
think it will make sense to have a common base controller which is not
the ApplicationController.

But this stuff is moving quickly, design-wise, and I wouldn’t want to
generally exclude the ApplicationController as an option. You guys
listed some drawbacks to consider and I totally agree that it’s
probably a bad idea in most situations. But there are also some
benefits like that the mere filename and classname largely contribute
to comprehensibility. (E.g. I generally don’t like when clearly
controller-related stuff is located somewhere in the lib/ dir and I
keep searching for it because I overlooked a single line at the top of
a controller …)

So, for now I think I can live with the anticipation that Rails will
change that filename (I think they really should!) and I will get the
feature for free :slight_smile:

Am 26.02.2008 um 11:58 schrieb James A.:

methods automatically mixed into the ApplicationController.
Will this be the case for everyone?
extracting the common functionality he needs into a

  • J *
    ~

Engine-Developers mailing list
removed_email_address@domain.invalid
http://lists.rails-engines.org/listinfo.cgi/engine-developers-rails-engines.org

–
sven fuchs removed_email_address@domain.invalid
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

#5

On 24/02/2008, Sven F. removed_email_address@domain.invalid wrote:

Is there a reason not to allow this feature for the application
controller? I’d like to move quite some stuff from an application
controller that I’m splitting up into engines … and there’s just no
more comprehensible place to use than plugin_name/app/controllers/
application.rb. :slight_smile:

The code mixing stuff only exists to make it simpler to override
methods within the application, but it sounds like you want to make
methods available to all controllers in your plugin by having those
methods automatically mixed into the ApplicationController.

This seems distinct from wanting to add methods defined within a
plugin to ApplicationController automatically.

In my usecase I can safely assume that all engines are designed to fit
together though and readability-wise it really makes most sense to use
ApplicationController here.

Will this be the case for everyone?

I fear that enabling this will lead to a variety of conflicts that are
difficult to control. Plugins from multiple sources are all adding
methods to the superclass of all application controllers in a way
that isn’t controllable by the application developer. Would it perhaps
be better that application developers (plugin ‘consumers’) have to
explicitly ‘opt-in’ to functionality that affects
ApplicationController directly?

I think the usecase Sven describes would be better solved by
extracting the common functionality he needs into a
non-ApplicationController class or module, and explicitly including it
into the controllers that require it.

… to bring a counter-point to my own points above, if Rails ever
adopts the ‘application_controller.rb’ name change proposal, this
behaviour will be available by default. Even in this case, I’d still
probably discourage defining application_controller files in any
plugins that are likely to be used outwith a very controlled
environment.