#2899 [PATCH] Rails 2.1.0 ActionView::Helpers::TextHelper#truncate - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2899 ✓resolved

[PATCH] Rails 2.1.0 ActionView::Helpers::TextHelper#truncate

Reported by guilherme | July 10th, 2009 @ 10:02 PM | in 2.3.4

I think it is a bug:

helper.truncate('TESTCASE',7,'.......') => "......." helper.truncate('TESTCASE',7,'........') => "TESTCAS........"

As said on doc:
------------------------------- ActionView::Helpers::TextHelper#truncate

 truncate(text, *args)

 Truncates a given +text+ after a given +:length+ if +text+ is
 longer than +:length+ (defaults to 30). The last characters will be
 replaced with the +:omission+ (defaults to "...").

Comments and changes to this ticket

Michael Koziarski August 3rd, 2009 @ 06:11 AM
- Tag changed from “2.1.0, actionview, texthelper” to “2.1.0, actionview, bugmash, texthelper”
You flagged this item as spam.
guilherme August 6th, 2009 @ 04:33 AM
- Tag changed from “2.1.0, actionview, bugmash, texthelper” to “2.1.0, 2.3.3, actionview, bugmash, texthelper”
the truncate on my opinion doesn't need to fit the text on the length especified by user. so i made this patch.

i've checked the 2.3.3 version and still have this bug. this patch is for 2.3.3 :)

sorry for my last post.. i didn't expected to crash my text :(
- truncate_fixed.diff 1.2 KB
You flagged this item as spam.
guilherme August 6th, 2009 @ 04:38 AM
ops, it was for edge. !
You flagged this item as spam.
jolohaga August 6th, 2009 @ 05:34 AM
I can confirm this bug.

Not only is the algorithm producing unpredictable results, but it uses :omission in the calculation. I agree with Guilherme that this doesn't seem the correct definition of truncate.
You flagged this item as spam.
jolohaga August 6th, 2009 @ 06:37 PM
Unless I'm missing something, it seems to me that the documentation is murky. Cases in point:

Truncates a given +text+ after a given :length if +text+ is longer than :length

(defaults to 30). The last characters will be replaced with the :omission (defaults to "...").

#

==== Examples

#

truncate("Once upon a time in a world far far away")

# => Once upon a time in a world f...

"Once upon a time in a world far far away".length == 40 "Once upon a time in a world f...".length == 32

In any case, length is incorrect according to the documentation.
It is not equal to 33 (if we decide not to use :omission in the truncation algorithm)
and is not it equal to 30 (if we do decide to use :omission in the algorithm).

#

truncate("Once upon a time in a world far far away", :length => 14)

# => Once upon a...

This example interprets the algorithm to include :omission, i.e. "Once upon a...".length == 14.

#

truncate("And they found that many people were sleeping better.", :length => 25, "(clipped)")

# => And they found that many (clipped)

This example interprets the algorithm to not include :omission. For example,
"And they found that many (clipped)".length == 34, "And they found that many ".length + "(clipped)".length = 25 + 9 = 34

#

truncate("And they found that many people were sleeping better.", :omission => "... (continued)", :length => 15)

# => And they found... (continued)

This case interprets the default length overrides the user provided :length.

I think truncate should be simply truncate and nothing else. The :omission string should be appended to whatever the result of the truncation is. I also think whenever :length is greater than the string's length, the length of the truncation should be zero and the output is just the :omission string. There doesn't not need to be any magic in this simple method.

We need to come to a consensus and be clear in the documentation as to what truncation means.
You flagged this item as spam.
jolohaga August 6th, 2009 @ 06:42 PM
Sorry for the headers. They resulted from copying documentation lines which were preceded by "#". If I could fix it I would. I'll know better next time.
Rizwan Reza August 8th, 2009 @ 01:18 PM
verified

+1 This is the behavior for truncate right now:
- I can verify that the length in the master branch is correct.
- The truncate method is taking omission into account when counting the length of the resulting string.
- The problem lies in omission. It's taking omission into account when I think omission should just be appended to the string after it has been truncated.
This behavior is happening both 2-3-stable and master.
You flagged this item as spam.
Steve St. Martin August 8th, 2009 @ 04:30 PM
- Title changed from “Rails 2.1.0 ActionView::Helpers::TextHelper#truncate” to “[PATCH] Rails 2.1.0 ActionView::Helpers::TextHelper#truncate”
+1 verified I've attached a patch that applies against 2.3.x and modified all tests to check for this behavior
- truncate_fix.diff 4.6 KB
Dan Pickett August 8th, 2009 @ 05:11 PM
+1 verified stevesmartin patch applies cleanly
Rizwan Reza August 8th, 2009 @ 05:11 PM
verified

-1 This patch only works in 2-3-stable. It would be great if this is also applicable to master.
You flagged this item as spam.
Steve St. Martin August 8th, 2009 @ 07:44 PM
updated patch with documentation changes to reflect expected behavior
- truncate_patch_2-3-stable.diff 1.6 MB
You flagged this item as spam.
Steve St. Martin August 8th, 2009 @ 07:44 PM
I've attached a patch that will apply cleanly to master, updated tests and documentation as well
- trucate_patch_master.diff 7.3 KB
You flagged this item as spam.
Steve St. Martin August 8th, 2009 @ 07:47 PM
Ooops, maybe we should diff against 2-3-stable instead of master for a 2-3-stable only patch. Sorry :P
- truncate_patch_2-3-stable.diff 6.4 KB
You flagged this item as spam.
Dan Croak August 8th, 2009 @ 09:16 PM
- Assigned user set to “Pratik”
+1 to stevestmartion's truncate_patch_2-3-stable.diff

It applies cleanly, rake test_action_pack runs green. Even covers 1.9 multibyte characters! Nice job.
Rizwan Reza August 8th, 2009 @ 09:28 PM
verified

+1 The new patch(es) apply cleanly.
You flagged this item as spam.
Elise Huard August 8th, 2009 @ 10:53 PM
verified
+1 expected behaviour.
You flagged this item as spam.
Rebecca Frankel August 8th, 2009 @ 10:55 PM
- Assigned user cleared.
verified

+1 The new patch apply cleanly to 2-3-stable the new code looked good to me
You flagged this item as spam.
Greg Sterndale August 8th, 2009 @ 11:02 PM
- Assigned user set to “Pratik”
+1 verified stevestmartin's truncate_patch_2-3-stable.diff patch applied, tests pass in 2-3-stable
nice job with examples in doc
Jeremy Kemper August 8th, 2009 @ 11:15 PM
These patches change truncate behavior - not ok for 2-3-stable.

Truncating to a specific length then appending the suffix is strange to me. Why would you expect this behavior? You're typically truncating to fit within an exact size.
Jeremy Kemper August 8th, 2009 @ 11:19 PM
- State changed from “new” to “invalid”
The truncation should definitely take the omission into account. The point is to restrict the truncated result to a specific size and not to exceed it.
Jeremy Kemper August 9th, 2009 @ 01:19 AM
- Tag changed from “2.1.0, 2.3.3, actionview, bugmash, texthelper” to “2.1.0, 2.3.3, actionview, texthelper”
You flagged this item as spam.
jolohaga August 9th, 2009 @ 05:06 AM
@Jeremy

The documentation isn't consistent with this assumption. My botched (full of headers) comment above discusses this point. With better documentation there wouldn't be confusion.

By the way, appending omission seems reasonable to me (and obviously to others as well), but I respect core's decision.
Jeremy Kemper August 9th, 2009 @ 08:41 PM
- Assigned user changed from “Pratik” to “Jeremy Kemper”
- State changed from “invalid” to “resolved”
- Milestone changed from “2.x” to “2.3.4”
2-3-stable: 679a0bf17fa9d74a4475753e9075bed8aea59e9a

master: 618771beb57e623e05f70fd31f3bdb0c04318017