Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

add taging ability to pluralize #5490

Closed
wants to merge 1 commit into from

8 participants

@lellisga

the pluralize can now return a tagged count. This allow us to easily manipulate the count number with jQuery and CSS

actionpack/lib/action_view/helpers/text_helper.rb
@@ -412,6 +419,12 @@ def set_cycle(name, cycle_object)
@_cycles = Hash.new unless defined?(@_cycles)
@_cycles[name] = cycle_object
end
+
+ # The pluralize helper need to tag the count number
+ # between two html tags.
+ def tag_count(tag,count)
+ tag == nil ? count : "<#{tag}>#{count}</#{tag}>"
@lest
lest added a note

I think that it's better to use content_tag(tag, count) instead of string interpolation.

Yeah, I agree with @lest , because with content_tag it will allow the use of classes and other html attributes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva

I think the api gets a bit cumbersome accepting 5 arguments, but I'm unsure about better ways now. An idea would be to accept a block, something like:

pluralize(1, 'foo') { |count| content_tag(:span, count) }

Another option is to create a separate method for that specific case. The method is so simple though, that I'm unsure about both approaches.

@sarcilav

@lellisga from the @carlosantoniodasilva 's comments I figured out that what about if I want to add html_options or a tag to the pluralized word.
So to let it works properly, for me looks like a better option to accept a block like pluralize(0, 'bar') { | count , word | ... } that let you do whatever you want

@lellisga

Wow! Thanks @carlosantoniodasilva and @sarcilav. I was thinking in tagging the word too but I then realize that the method will have to accept a lot more arguments and the word can still be accessed using CSS or JQ. I'll go ahead a do it with the block (have to read a little bit about that :D). Thanks again!

@lellisga

@carlosantoniodasilva and @sarcilav . I really don't know how good this can be put it works! https://gist.github.com/2088406

@carlosantoniodasilva

@lellisga sounds good :). I just don't think you need to check for block arity. If a block is given, yield both count and word and let the user do the work, otherwise use the current logic. This way you wouldn't need the block argument anymore.

@lellisga

sorry, didn';t see the comment :(

@lellisga

@carlosantoniodasilva but I still have to ask in order to pluralize the word

@rafaelfranca

Hey @lellisga I agree with @carlosantoniodasilva. You don't need to check the block arity. If the user need to customize the count but not the word he can do something like this:

pluralize(3, 'person', 'users') { |count, word| "#{content_tag(:b, count)} #{word}"}
actionpack/lib/action_view/helpers/text_helper.rb
@@ -189,8 +190,19 @@ def excerpt(text, phrase, *args)
#
# pluralize(0, 'person')
# # => 0 people
- def pluralize(count, singular, plural = nil)
- "#{count || 0} " + ((count == 1 || count =~ /^1(\.0+)?$/) ? singular : (plural || singular.pluralize))
+ #
+ # pluralize(1, 'person'){|count| content_tag(:em,count) }}
+ # # => <em>1</em> person
+ #
+ # pluralize(3, 'person', 'users'){|count, word| "#{content_tag(:b,count)} #{content_tag(:span,word)}"}
+ # # => <b>3</b> <span>users</span>
+ def pluralize(count, singular, plural = nil,&block)
+ word = ((count == 1 || count =~ /^1(\.0+)?$/) ? singular : (plural || singular.pluralize))
+ if block_given?
+ block.arity == 2 ? "#{yield(count,word)}" : "#{yield(count,word)} #{word}"
@rafaelfranca Owner

Some considerations about code style. Put one space after the ,. Like this:

yield(count, word)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
actionpack/lib/action_view/helpers/text_helper.rb
@@ -189,8 +190,19 @@ def excerpt(text, phrase, *args)
#
# pluralize(0, 'person')
# # => 0 people
- def pluralize(count, singular, plural = nil)
- "#{count || 0} " + ((count == 1 || count =~ /^1(\.0+)?$/) ? singular : (plural || singular.pluralize))
+ #
+ # pluralize(1, 'person'){|count| content_tag(:em,count) }}
+ # # => <em>1</em> person
+ #
+ # pluralize(3, 'person', 'users'){|count, word| "#{content_tag(:b,count)} #{content_tag(:span,word)}"}
@rafaelfranca Owner

Put a space before the { when it is a block, after it and before the }. Like this:

pluralize(3, 'person', 'users'){ |count, word| "#{content_tag(:b, count)} #{content_tag(:span, word)}" }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
actionpack/test/template/text_helper_test.rb
@@ -256,6 +256,9 @@ def test_pluralization
assert_equal("10 buffaloes", pluralize(10, "buffalo"))
assert_equal("1 berry", pluralize(1, "berry"))
assert_equal("12 berries", pluralize(12, "berry"))
+ assert_equal("<b>1</b> count", pluralize(1,"count") { |count| content_tag(:b,count) })
+ assert_equal("<span>2</span> counts", pluralize(2,"count"){ |count| content_tag(:span,count) })
+ assert_equal("<p>1</p> <span>count</span>", pluralize(1,"count") {|count,word,| "#{content_tag(:p,count)} #{content_tag(:span,word)}" })
@rafaelfranca Owner

The tests should follow the same code style too. Also this test had one more , after the word block argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lellisga

@rafaelfranca Thanks a lot!

@carlosantoniodasilva

@lellisga hey, sounds great. I'd just say you don't need to use string interpolation here: "#{yield(count, word)}", you'd be fine with just the yield call: yield(count, word). Other than that, I think you just need to squash everything on a single commit :). Thanks.

actionpack/lib/action_view/helpers/text_helper.rb
@@ -189,8 +190,15 @@ def excerpt(text, phrase, *args)
#
# pluralize(0, 'person')
# # => 0 people
+ #
+ # pluralize(1, 'person'){ |count, word| "#{content_tag(:em, count)} #{word} }
+ # # => <em>1</em> person
+ #
+ # pluralize(3, 'person', 'users'){ |count, word| "#{content_tag(:b, count)} #{content_tag(:span,word)}" }
+ # # => <b>3</b> <span>users</span>
@rafaelfranca Owner

And in this line you still have content_tag(:span,word) without the space after the ,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lellisga

@carlosantoniodasilva done!, thanks again

@rafaelfranca

@lellisga thank you!

@lellisga

@rafaelfranca Thank to you

@pixeltrix
Owner

@lellisga how does wrapping the count make it easier to update via jQuery - you still might need to changed the word? For example if you go from '1 item' to '2 items'.

@lellisga

@pixeltrix Sure but if that's the case is much cheaper to get the number and do your logic instead of doing a new request, that's not all, having both tagged makes it easier to jquery to find the count or the word, without the tag it will get the complete phrase thus you'll have to do regex to get the count or the word

@lellisga

any word on this?

@pixeltrix
Owner

Sorry for not getting back sooner. We discussed it in the Campfire room and the consensus was that it was an application level concern - thanks for your contribution though.

@pixeltrix pixeltrix closed this
@lellisga

(Y) thanks to u

@graysonwright

Could we revisit this topic?

This seems like a very light commit, with only one SLOC changed, and I believe that it fixes functionality that is now very limited.

The pluralize method takes a number and a string, and returns only a string -- once you call the method, you constrain yourself to working with a string from that point forward. If you want to further manipulate the word or the number, the method becomes useless to you.

The use case of tagging the number and word separately came up. For example, if you want to manipulate the number with styling, as in this image:
tagged_numbers

something like the following must currently be done:

<span class='number-emph'>
  <%= @user.projects.count %>
</span><br/>
<%= @user.projects.one? 'Project' : 'Projects' %>

The last line -- checking to see if the count is one, then selecting a version of a word to match, is almost exactly what's being done in the pluralize method already -- it makes sense to expand the method to include this use case.

Manipulating the page with javascript, or scraping pages for data is also made a lot easier by being able to tag the word and number separately.

The method becomes much more effective and powerful when you have the option of simply transforming the word to match the count, instead of being forced to concatenate the two into a string.

@dalibor

@graysonwright you can just create a helper method that wraps pluralize + html tag.

@graysonwright

Yeah, I suppose that would work. It just seems like it's really close to what the pluralize method is already doing.

I guess I'm confused about the criteria for accepting the feature into Rails. @pixeltrix mentioned that it was an application-level concern, but it seems to me like it's a natural extension of the pluralize helper that's already in Rails. What's the criteria for making that decision?

Thanks for your help.

@dalibor

@graysonwright I'm not sure if there is an exact criteria, but my opinion is that the framework should do the hard stuff for you, and leave the easy stuff to you.

Other thing for this specific case is that pluralize method should do only one thing - pluralization. If I want html tag wrapping I would introduce another method like pluralize_with_wrap. But since it's so easy to write that helper myself, it's best to prevent the framework from getting bloated and keep it easy to maintain. ;)

@pixeltrix
Owner

@graysonwright there are basically two criteria for whether a new feature is added to Rails:

  1. Can it be used within the Rails codebase? This covers most of Active Support which is not a general Ruby extension library like Ruby Facets, but specifically for Rails or developing web applications.

  2. Is it a generalized feature and be of use to a large proportion of Rails users? For example we have a generic <video> tag helper but not a helper for generating embedded YouTube videos even though it might be used by a large number of developers.

@graysonwright

Makes sense.

I appreciate the explanation. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2012
  1. @lellisga
This page is out of date. Refresh to see the latest.
View
15 actionpack/lib/action_view/helpers/text_helper.rb
@@ -177,7 +177,8 @@ def excerpt(text, phrase, *args)
# Attempts to pluralize the +singular+ word unless +count+ is 1. If
# +plural+ is supplied, it will use that when count is > 1, otherwise
- # it will use the Inflector to determine the plural form
+ # it will use the Inflector to determine the plural form. It also
+ # supports tagging the count number.
#
# ==== Examples
# pluralize(1, 'person')
@@ -191,8 +192,18 @@ def excerpt(text, phrase, *args)
#
# pluralize(0, 'person')
# # => 0 people
+ #
+ # pluralize(1, 'person'){ |count, word| "#{content_tag(:em, count)} #{word} }
+ # # => <em>1</em> person
+ #
+ # pluralize(3, 'person', 'users'){ |count, word| "#{content_tag(:b, count)} #{content_tag(:span, word)}" }
+ # # => <b>3</b> <span>users</span>
+ #
+ # pluralize(3, 'person', 'users'){ |count, word| "#{content_tag(:b, count, :class => 'count')} #{content_tag(:span, word, :class => 'word')}" }
+ # # => <b class="count">3</b> <span class="word">users</span>
def pluralize(count, singular, plural = nil)
- "#{count || 0} " + ((count == 1 || count =~ /^1(\.0+)?$/) ? singular : (plural || singular.pluralize))
+ word = ((count == 1 || count =~ /^1(\.0+)?$/) ? singular : (plural || singular.pluralize))
+ block_given? ? yield(count, word) : "#{count || 0} #{word}"
end
# Wraps the +text+ into lines no longer than +line_width+ width. This method
View
6 actionpack/test/template/text_helper_test.rb
@@ -256,6 +256,12 @@ def test_pluralization
assert_equal("10 buffaloes", pluralize(10, "buffalo"))
assert_equal("1 berry", pluralize(1, "berry"))
assert_equal("12 berries", pluralize(12, "berry"))
+ assert_equal("<b>1</b> count", pluralize(1, "count"){ |count, word| "#{content_tag(:b, count)} #{word}"} )
+ assert_equal("<span>2</span> counts", pluralize(2, "count"){ |count, word| "#{content_tag(:span, count)} #{word}" })
+ assert_equal("<p>1</p> <span>count</span>", pluralize(1, "count"){ |count, word| "#{content_tag(:p, count)} #{content_tag(:span, word)}" })
+ assert_equal("1 <span>count</span>", pluralize(1, "count"){ |count, word| "#{count} #{content_tag(:span, word)}" })
+ assert_equal("<b class=\"strong\">1</b> count", pluralize(1, "count"){ |count, word| "#{content_tag(:b, count, :class => 'strong')} #{word}"} )
+ assert_equal("<b class=\"count\">1</b> <span class=\"word\">count</span>", pluralize(1, "count"){ |count, word| "#{content_tag(:b, count, :class => 'count')} #{content_tag(:span, word, :class => 'word')}"} )
end
def test_cycle_class
Something went wrong with that request. Please try again.