Hey there! I've been working on this patch and I would like to read your opinions about it.
Basically, the `Rails::Generators::Base#indent` method can be replaced with the new `Rails::Generators::Actions#optimize_indentation` introduced in 8bf0967 <https://github.com/ricardotk002/rails/commit/8bf09678b38f5ec4d6c120d143042b7ab14a370b>. I think that it would be beneficial to avoid duplication and use a single version. However, the newly introduced method is slightly less efficient as the benchmark I did shows and that's why I'm unsure to create a new PR for this. I'm attaching the patch with the change and here's also the commit in GitHub https://github.com/ricardotk002/rails/commit/a08d0de7c671814c2d7735c88d0961bccd7fa8b7 Any feedback will be appreciated, thanks! -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at https://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
>From a08d0de7c671814c2d7735c88d0961bccd7fa8b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Di=CC=81az?= <ricardotk...@gmail.com> Date: Fri, 4 May 2018 00:29:59 -0500 Subject: [PATCH] Remove redundant Rails::Generators::Base#indent method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although, `Rails::Generators::Actions#optimize_indentation` is less efficient as the following benchmark shows. This is because it uses `String#strip_heredoc` which is a little bit slower. It also uses `String#indent` which is less efficient than `Rails::Generators::Base#indent` Benchmark: https://gist.github.com/ricardotk002/df04a855b7abb4c84b291f91c19e754e ================================= Single Line ================================== Warming up -------------------------------------- indent 42.142k i/100ms optimize_indentation 21.654k i/100ms String#indent 25.251k i/100ms Calculating ------------------------------------- indent 543.114k (± 7.5%) i/s - 2.739M in 5.069267s optimize_indentation 241.681k (± 8.1%) i/s - 1.213M in 5.046086s String#indent 283.356k (± 8.2%) i/s - 1.414M in 5.021961s Comparison: indent: 543114.4 i/s String#indent: 283355.8 i/s - 1.92x slower optimize_indentation: 241681.0 i/s - 2.25x slower ================================ Multiple Lines ================================ Warming up -------------------------------------- indent 4.252k i/100ms optimize_indentation 1.707k i/100ms String#indent 5.212k i/100ms Calculating ------------------------------------- indent 42.786k (± 6.4%) i/s - 216.852k in 5.091541s optimize_indentation 17.071k (± 5.5%) i/s - 85.350k in 5.014778s String#indent 52.654k (±14.5%) i/s - 255.388k in 5.066158s Comparison: String#indent: 52654.4 i/s indent: 42785.8 i/s - same-ish: difference falls within error optimize_indentation: 17070.8 i/s - 3.08x slower ================================= Short String ================================= Warming up -------------------------------------- indent 45.968k i/100ms optimize_indentation 23.572k i/100ms String#indent 28.039k i/100ms Calculating ------------------------------------- indent 544.232k (± 6.4%) i/s - 2.712M in 5.002628s optimize_indentation 251.062k (±11.1%) i/s - 1.249M in 5.035213s String#indent 299.637k (± 8.6%) i/s - 1.514M in 5.087353s Comparison: indent: 544232.3 i/s String#indent: 299636.5 i/s - 1.82x slower optimize_indentation: 251061.7 i/s - 2.17x slower =============================== Very Long String =============================== Warming up -------------------------------------- indent 34.244k i/100ms optimize_indentation 46.833k i/100ms String#indent 17.085k i/100ms Calculating ------------------------------------- indent 380.103k (± 6.4%) i/s - 1.918M in 5.064455s optimize_indentation 535.376k (± 4.1%) i/s - 2.716M in 5.082079s String#indent 176.810k (± 8.4%) i/s - 888.420k in 5.063517s Comparison: optimize_indentation: 535376.3 i/s indent: 380102.7 i/s - 1.41x slower String#indent: 176809.9 i/s - 3.03x slower --- railties/lib/rails/generators/base.rb | 7 +------ .../lib/rails/generators/rails/controller/controller_generator.rb | 6 +++--- .../generators/rails/resource_route/resource_route_generator.rb | 6 +++--- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/railties/lib/rails/generators/base.rb b/railties/lib/rails/generators/base.rb index 5523a3f659..701cf2bbf4 100644 --- a/railties/lib/rails/generators/base.rb +++ b/railties/lib/rails/generators/base.rb @@ -284,13 +284,8 @@ def module_namespacing(&block) # :doc: concat(content) end - def indent(content, multiplier = 2) # :doc: - spaces = " " * multiplier - content.each_line.map { |line| line.blank? ? line : "#{spaces}#{line}" }.join - end - def wrap_with_namespace(content) # :doc: - content = indent(content).chomp + content = optimize_indentation(content, 2).chomp "module #{namespace.name}\n#{content}\nend\n" end diff --git a/railties/lib/rails/generators/rails/controller/controller_generator.rb b/railties/lib/rails/generators/rails/controller/controller_generator.rb index eb75e7e661..eff7eceab6 100644 --- a/railties/lib/rails/generators/rails/controller/controller_generator.rb +++ b/railties/lib/rails/generators/rails/controller/controller_generator.rb @@ -51,7 +51,7 @@ def generate_routing_code # namespace :foo do # namespace :bar do regular_class_path.each do |ns| - lines << indent("namespace :#{ns} do\n", depth * 2) + lines << optimize_indentation("namespace :#{ns} do", depth * 2) depth += 1 end @@ -59,7 +59,7 @@ def generate_routing_code # get 'baz/index' # get 'baz/show' actions.each do |action| - lines << indent(%{get '#{file_name}/#{action}'\n}, depth * 2) + lines << optimize_indentation(%{get '#{file_name}/#{action}'}, depth * 2) end # Create `end` ladder @@ -67,7 +67,7 @@ def generate_routing_code # end until depth.zero? depth -= 1 - lines << indent("end\n", depth * 2) + lines << optimize_indentation("end", depth * 2) end lines.join diff --git a/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb b/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb index 9a92991efe..b19eafb3a5 100644 --- a/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb +++ b/railties/lib/rails/generators/rails/resource_route/resource_route_generator.rb @@ -24,21 +24,21 @@ def add_resource_route # namespace :foo do # namespace :bar do regular_class_path.each do |ns| - lines << indent("namespace :#{ns} do\n", depth * 2) + lines << optimize_indentation("namespace :#{ns} do", depth * 2) depth += 1 end # inserts the primary resource # Create route # resources 'products' - lines << indent(%{resources :#{file_name.pluralize}\n}, depth * 2) + lines << optimize_indentation(%{resources :#{file_name.pluralize}}, depth * 2) # Create `end` ladder # end # end until depth.zero? depth -= 1 - lines << indent("end\n", depth * 2) + lines << optimize_indentation("end", depth * 2) end route lines.join -- 2.15.1
