Deprecate safe_level of ERB.new in Ruby 2.6 #594
Conversation
koic
force-pushed
the
deprecate_safe_level_of_erb_new_in_ruby_2_6
branch
3 times, most recently
from
ca4881b
to
9be0a15
Compare
|
I would like this pull request merged since there are a bunch of warnings at Rails CI with Ruby 2.6. https://travis-ci.org/rails/rails/jobs/357183623 There are two failures with Ruby 2.6, then I have opened #595, For 1.8.7 failures at https://travis-ci.org/erikhuda/thor/jobs/357338902 I simply wanted to drop Ruby 1.8.7 support. |
|
It looks like this pull request failure with Ruby 1.8.7 is due to "newer" regular expression syntax Failure/Error: require "thor/actions/file_manipulation"
SyntaxError:
/home/travis/build/yahonda/thor/spec/../lib/thor/actions/file_manipulation.rb:120: undefined (?...) sequence: /\Aerb\.rb \[(?<version>[^ ]+) /I have made a quick and dirty workaround to support Ruby 1.8.7. yahonda@8157118 Now CI is green for all supported versions of Ruby https://travis-ci.org/yahonda/thor/builds/36489840 @koic Thank you. |
koic
force-pushed
the
deprecate_safe_level_of_erb_new_in_ruby_2_6
branch
from
9be0a15
to
708365d
Compare
|
@yahonda Thanks for your investigation. I replaced it with regular expression that you showed 🙏 |
| @@ -117,7 +117,13 @@ def template(source, *args, &block) | |||
| context = config.delete(:context) || instance_eval("binding") | |||
|
|
|||
| create_file destination, nil, config do | |||
| content = CapturableERB.new(::File.binread(source), nil, "-", "@output_buffer").tap do |erb| | |||
| match = ERB.version.match(/(\d\.\d.\d)/) | |||
There was a problem hiding this comment.
My previous commit did not have a necessary backslash for the second "."
match = ERB.version.match(/(\d\.\d\.\d)/)Here the dot should be actual dot character not any one character.
There was a problem hiding this comment.
I couldn't write a more correctly regular expression because of lack of self review 💦 I fixed it.
There was a problem hiding this comment.
Thank you for the update. I believe we did our best.
The interface of `ERB.new` will change from Ruby 2.6. > Add :trim_mode and :eoutvar keyword arguments to ERB.new. > Now non-keyword arguments other than first one are softly deprecated > and will be removed when Ruby 2.5 becomes EOL. [Feature #14256] https://github.com/ruby/ruby/blob/2311087b685e8dc0f21f4a89875f25c22f5c39a9/NEWS#stdlib-updates-outstanding-ones-only The following address is related Ruby's commit. ruby/ruby@cc777d0 This PR uses `ERB.version` to switch `ERB.new` interface. Because Thor supports multiple Ruby versions, it need to use the appropriate interface. Using `ERB.version` instead of `RUBY_VERSON` is based on the following patch. ruby/ruby#1826 This patch is built into Ruby. ruby/ruby@40db89c
koic
force-pushed
the
deprecate_safe_level_of_erb_new_in_ruby_2_6
branch
from
708365d
to
70021d0
Compare
|
Rails railties testing against ruby-head (aka Ruby 2.6.0 dev) has been failing due to And the biggest number of warnings will be fixed by this pull request. Here are number of warnings. $ grep 'warning:' log.txt | sort | uniq -c | sort -rn | head -n 5
6092 /home/travis/.rvm/gems/ruby-head/gems/thor-0.20.0/lib/thor/actions/file_manipulation.rb:120: warning: Passing trim_mode with the 3rd argument of ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.
6092 /home/travis/.rvm/gems/ruby-head/gems/thor-0.20.0/lib/thor/actions/file_manipulation.rb:120: warning: Passing eoutvar with the 4th argument of ERB.new is deprecated. Use keyword argument like ERB.new(str, eoutvar: ...) instead.
5949 /home/travis/.rvm/gems/ruby-head/gems/thor-0.20.0/lib/thor/actions/file_manipulation.rb:120: warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
183 /home/travis/.rvm/gems/ruby-head/gems/sqlite3-1.3.13/lib/sqlite3/pragmas.rb:34: warning: mismatched indentations at 'else' with 'case' at 21
116 /home/travis/.rvm/gems/ruby-head/gems/sqlite3-1.3.13/lib/sqlite3/pragmas.rb:26: warning: mismatched indentations at 'else' with 'case' at 23
$Thank you. |
| @@ -117,7 +117,13 @@ def template(source, *args, &block) | |||
| context = config.delete(:context) || instance_eval("binding") | |||
|
|
|||
| create_file destination, nil, config do | |||
| content = CapturableERB.new(::File.binread(source), nil, "-", "@output_buffer").tap do |erb| | |||
| match = ERB.version.match(/(\d\.\d\.\d)/) | |||
There was a problem hiding this comment.
This won't work on versions with more numerics. While it's not a problem now, it could cause wierd errors in the future.
example
'2.11.0'.match(/(\d\.\d\.\d)/) -> nilan old behaviour is used even if '2.11.0' is newer than '2.2.0'
'2.11.0' >= '2.2.0' -> false
Fix introduced in rails#594 misidentifies versions with more numerics. Version "2.11.0", for example, would be called using the deprecated API. Long live ERB.
The interface of
ERB.newwill change from Ruby 2.6.https://github.com/ruby/ruby/blob/2311087b685e8dc0f21f4a89875f25c22f5c39a9/NEWS#stdlib-updates-outstanding-ones-only
The following address is related Ruby's commit.
ruby/ruby@cc777d0
This PR uses
ERB.versionto switchERB.newinterface. Because Thor supports multiple Ruby versions, it need to use the appropriate interface.Using
ERB.versioninstead ofRUBY_VERSONis based on the following patch.ruby/ruby#1826
This patch is built into Ruby.
ruby/ruby@40db89c