Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Decoupling ActiveSupport from ActionView #5675

Closed
wants to merge 2 commits into from

5 participants

@amutz

I'm resubmitting this pull request on master, since my previous pull request was on 3-1-stable.

I'm interested in using the ActiveSupport::Testing::Performance module and was surprised to see that it requires and calls code from ActionView. Is this intentional?

I'd like to use this module in a context without ActionView, so I'm submitting a pull request that decouples ActiveSupport from ActionView.

@josevalim
Owner

If it is requiring methods from action view, we could alternatively rewrite it or move the action view methods to AS. /cc @fxn

@carlosantoniodasilva

Just linking to the other pull request #4952, and following the issue.

@amutz

Hi, I can implement a different solution to the problem if you like. Would you like me to reimplement the logic from NumberHelper in AS::Testing::Performance or would you like me to move the NumberHelper code over to AS (and include it in AV::Helpers::NumberHelpers)?

Thanks!

@josevalim
Owner

I am leaving this decision to @fxn :)

@rafaelfranca rafaelfranca was assigned
@amutz

The most recent addition to the pull request was an oversight. I will fix it.

But since @fxn is making the call here, how would you like this implemented? I'd still like to decouple activesupport from actionview. How would you like this done? Should I clean up the original pull request, or should I reimplement it one of the ways suggested by @josevalim ?

@carlosantoniodasilva

@amutz I want to add that there's some idea to extract some of the date/time/number AV helpers to ActiveSupport, and that would probably solve this problem as well by being able to use something like 1.human_size.

#3214

@fxn
Owner

Hey guys, really busy days lately. Just a heads up to say I have this in my queue.

@amutz

I've implemented the approach of moving the NumberHelper methods to ActiveSupport:

#6315

Let me know if I can help move this forward in any way.

Thanks!

@carlosantoniodasilva

I'm closing this in favor of #6526.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 30, 2012
  1. @amutz
Commits on Apr 10, 2012
  1. @amutz

    Merge branch 'master' of git://github.com/rails/rails into decouple_a…

    amutz authored
    …ctivesupport_from_actionview_on_master
This page is out of date. Refresh to see the latest.
View
11 actionpack/lib/action_dispatch/testing/performance_test.rb
@@ -1,4 +1,15 @@
require 'active_support/testing/performance'
+require 'action_view/helpers/number_helper'
+
+ActiveSupport::Testing::Performance::Metrics::Amount.formatter = Proc.new() do |measurement|
+ include ActionView::Helpers::NumberHelper
+ number_with_delimiter(measurement.floor)
+end
+
+ActiveSupport::Testing::Performance::Metrics::DigitalInformationUnit.formatter = Proc.new() do |measurement|
+ include ActionView::Helpers::NumberHelper
+ number_to_human_size(measurement, :precision => 2)
+end
module ActionDispatch
# An integration test that runs a code profiler on your test methods.
View
17 activesupport/lib/active_support/testing/performance.rb
@@ -3,7 +3,7 @@
require 'active_support/concern'
require 'active_support/core_ext/class/delegating_attributes'
require 'active_support/core_ext/string/inflections'
-require 'action_view/helpers/number_helper'
+require 'active_support/core_ext/class/attribute'
module ActiveSupport
module Testing
@@ -195,9 +195,8 @@ def self.[](name)
end
class Base
- include ActionView::Helpers::NumberHelper
-
attr_reader :total
+ class_attribute :formatter
def initialize
@total = 0
@@ -239,13 +238,21 @@ def format(measurement)
class Amount < Base
def format(measurement)
- number_with_delimiter(measurement.floor)
+ if self.class.formatter.present?
+ self.class.formatter.call(measurement.floor)
+ else
+ measurement.floor
+ end
end
end
class DigitalInformationUnit < Base
def format(measurement)
- number_to_human_size(measurement, :precision => 2)
+ if self.class.formatter.present?
+ self.class.formatter.call(measurement)
+ else
+ "#{measurement} Bytes"
+ end
end
end
Something went wrong with that request. Please try again.