This project is archived and is in readonly mode.

#2899 ✓resolved
guilherme

[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

    Michael Koziarski August 3rd, 2009 @ 06:11 AM

    • Tag changed from 2.1.0, actionview, texthelper to 2.1.0, actionview, bugmash, texthelper
  • guilherme

    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 :(

  • guilherme

    guilherme August 6th, 2009 @ 04:38 AM

    ops, it was for edge. !

  • jolohaga

    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.

  • jolohaga

    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.

  • jolohaga

    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

    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.

  • Steve St. Martin

    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

  • Dan Pickett

    Dan Pickett August 8th, 2009 @ 05:11 PM

    +1 verified stevesmartin patch applies cleanly

  • Rizwan Reza

    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.

  • Steve St. Martin

    Steve St. Martin August 8th, 2009 @ 07:44 PM

    updated patch with documentation changes to reflect expected behavior

  • Steve St. Martin

    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

  • Steve St. Martin

    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

  • Dan Croak

    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

    Rizwan Reza August 8th, 2009 @ 09:28 PM

    verified

    +1 The new patch(es) apply cleanly.

  • Elise Huard

    Elise Huard August 8th, 2009 @ 10:53 PM

    verified
    +1 expected behaviour.

  • Rebecca Frankel

    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

  • Greg Sterndale

    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

    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

    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

    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
  • jolohaga

    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

    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

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages