Fix for external generators extend Erb::Generators #14132

Closed
wants to merge 5 commits into
from

Projects

None yet

7 participants

@afair
afair commented Feb 20, 2014

HAML and probably other generators extend this class and invoke filename_with_extensions with the old signature (without format). This makes the second argument optional and defaults it to the #format method which could be overridden as well.

@afair afair Fix for external generators extend Erb::Generators
HAML and probably other generators extend this class and invoke filename_with_extensions with the old signature (without format). This makes the second argument optional and defaults it to the #format method which could be overridden as well.
38c9de7
@afair
afair commented on 38c9de7 Feb 21, 2014

The problem occurs with Rails 4.1.0.rc1 with haml-rails 0.5.3. Rails 4.1.0.beta1 did not have the problem.

@indirect indirect referenced this pull request in indirect/haml-rails Feb 21, 2014
Closed

Error generating view with Rails 4.1 #66

@arthurnn arthurnn and 1 other commented on an outdated diff Feb 21, 2014
railties/lib/rails/generators/erb.rb
@@ -17,8 +17,8 @@ def handler
:erb
end
- def filename_with_extensions(name, format)
- [name, format, handler].compact.join(".")
+ def filename_with_extensions(name, format_override=nil)
@arthurnn
arthurnn Feb 21, 2014 Ruby on Rails member

how about we do this instead:

def filename_with_extensions(name, format_override=self.format)

So we wont need the || check in the next line

@afair
afair Feb 21, 2014

Great, I didn't know you could set a default with a method call. I'll update it.

@mdesantis

It breaks Slim too, FYI

@mdesantis mdesantis referenced this pull request in slim-template/slim-rails Feb 21, 2014
Closed

Rails 4.1 #40

@robin850 robin850 added this to the 4.1.0 milestone Feb 22, 2014
@razum2um

breaks rabl-generators as well

@carlosantoniodasilva carlosantoniodasilva commented on an outdated diff Feb 24, 2014
railties/lib/rails/generators/erb.rb
@@ -17,8 +17,8 @@ def handler
:erb
end
- def filename_with_extensions(name, format)
- [name, format, handler].compact.join(".")
+ def filename_with_extensions(name, format_override=self.format)
@carlosantoniodasilva
carlosantoniodasilva Feb 24, 2014 Ruby on Rails member

Minor thing: please add space surrounding =, and I believe you don't need to use self since you are using a different variable name.

@carlosantoniodasilva

Is there a way to provide a minor test case so that we ensure won't break it again? Thanks.

@kevinelliott

👍 for this fix.

afair added some commits Feb 27, 2014
@afair afair Adds regression test for filename_with_extensions default
Also renames the format_override to simply format in the fix.
99fd155
@afair afair Reverts test case
The test case did not work. I don't know how to isolate this as a test.

Can anyone help?
51f7de8
@afair afair Formatting change to trigger CI
Previous build failed with a memcache issue on one run. I couldn't find
how to trigger this to run again without a push.
025a26b
@carlosantoniodasilva carlosantoniodasilva added a commit that closed this pull request Mar 4, 2014
@afair afair Fix for external generators extend Erb::Generators
HAML and probably other generators extend this class and invoke
filename_with_extensions with the old signature (without format).
This makes the second argument optional and defaults it to the #format
method which could be overridden as well.

Closes #14132.
3624ff7
@carlosantoniodasilva carlosantoniodasilva added a commit that referenced this pull request Mar 5, 2014
@afair afair Fix for external generators extend Erb::Generators
HAML and probably other generators extend this class and invoke
filename_with_extensions with the old signature (without format).
This makes the second argument optional and defaults it to the #format
method which could be overridden as well.

Closes #14132.
4ca8180
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment