This project is archived and is in readonly mode.

#6522 open
Clemens Kofler

distance_of_time_in_words / time_ago_in_words shouldn't use named arguments

Reported by Clemens Kofler | March 4th, 2011 @ 04:45 PM | in 3.x

In contrast to most other view helpers, distance_of_time_in_words and time_ago_in_words still use a named argument (include_seconds). I propose to get things in line with the rest and pass :include_seconds => true/false as part of an options hash.

# old
distance_of_time_in_words(19.seconds.ago, Time.current, true) # => less than 20 seconds
time_ago_in_words(19.seconds.ago, true) # => less than 20 seconds

# new
distance_of_time_in_words(19.seconds.ago, Time.current, :include_seconds => true) # => less than 20 seconds
time_ago_in_words(19.seconds.ago, :include_seconds => true) # => less than 20 seconds

Although there's a little more typing, I'd argue that it's less surprising.

A patch is attached. All tests are green and the patch also adds new tests (same as the old ones with just the argument swapped out for a hash). I've also updated the documentation and added a deprecation warning.

Comments and changes to this ticket

  • Dan Pickett

    Dan Pickett March 13th, 2011 @ 05:32 PM

    • Milestone set to 3.x
    • Assigned user set to “Dan Pickett”
    • Importance changed from “” to “Low”

    I'm +1 on this and it applies cleanly to master, but it creates a lot of noise in the test suite currently. Can you update the test suite to comply with your new contract?

  • Clemens Kofler

    Clemens Kofler March 13th, 2011 @ 09:24 PM

    Sure – I just left the old tests untouched to ensure things still work. They are removed in the attached patch. I've also rebased against the current master.

  • Dan Pickett

    Dan Pickett March 14th, 2011 @ 04:09 PM

    • State changed from “new” to “open”
    • Assigned user changed from “Dan Pickett” to “Santiago Pastorino”

    a model patch. Applies cleanly to master and appears well documented.

    Thanks Clemens. Assigning to Santiago for his review and signoff.

  • Clemens Kofler

    Clemens Kofler May 2nd, 2011 @ 03:57 PM

    Any news on that one? Not that there's any time pressure on it but I don't want it to become stale. :)

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>

Pages