Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new directive: day with suffix #463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yangel
Copy link

@yangel yangel commented Aug 29, 2018

Summary of changes

Addressed issue described in #426

Checklist

  • New functions have typespecs, changed functions were updated
  • Same for documentation, including moduledocs
  • Tests were added or updated to cover changes
  • Commits were squashed into a single coherent commit

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.3%) to 69.844% when pulling b74c5b6 on yangel:day_suffix into 4a163a1 on bitwalker:master.

@bitwalker
Copy link
Owner

These type of numbers are called ordinals, so I prefer to use that terminology when possible. Since ordinals can come with a suffix or without in written form, this can still be somewhat confusing, but from a maintenance point of view, I'd like to clarify things in here by saying "ordinal suffix" rather than "day suffix", since the latter isn't clear to me what it means on first read.

Rather than add a new directive for this though, I think it may make more sense to add it as a flag (i.e. if the current directive is represented by an ordinal number, require the suffix in printed form - if the current directive is not an ordinal number, then the flag is invalid, and either ignored or results in an error). This means we can easily add it to both the default and strftime formats by introducing a corresponding flag, and would support arbitrary ordinals with suffixes (e.g. if you wanted a format that says something like "the 10th hour of the 3rd day of March" or something like that).

I haven't yet thought through what the flag would be in the two formats, but let's discuss the approach first, and we can hash that out.

@zorn
Copy link

zorn commented May 22, 2020

I tried to use the code in this PR and noticed issues with some numbers. I eventually rewrote it like this:

  def ordinal_suffix(value) when value in [1, 21, 31] do
    "st"
  end

  def ordinal_suffix(value) when value in [2, 22] do
    "nd"
  end

  def ordinal_suffix(value) when value in [3, 23] do
    "rd"
  end

  def ordinal_suffix(value) do
    "th"
  end

With tests:

  describe "ordinal_suffix/1" do
    test "valid data scenarios" do
      assert EventDateFormatter.ordinal_suffix(1) == "st"
      assert EventDateFormatter.ordinal_suffix(2) == "nd"
      assert EventDateFormatter.ordinal_suffix(3) == "rd"
      assert EventDateFormatter.ordinal_suffix(4) == "th"
      assert EventDateFormatter.ordinal_suffix(5) == "th"
      assert EventDateFormatter.ordinal_suffix(6) == "th"
      assert EventDateFormatter.ordinal_suffix(7) == "th"
      assert EventDateFormatter.ordinal_suffix(8) == "th"
      assert EventDateFormatter.ordinal_suffix(9) == "th"
      assert EventDateFormatter.ordinal_suffix(10) == "th"
      assert EventDateFormatter.ordinal_suffix(11) == "th"
      assert EventDateFormatter.ordinal_suffix(12) == "th"
      assert EventDateFormatter.ordinal_suffix(13) == "th"
      assert EventDateFormatter.ordinal_suffix(14) == "th"
      assert EventDateFormatter.ordinal_suffix(15) == "th"
      assert EventDateFormatter.ordinal_suffix(16) == "th"
      assert EventDateFormatter.ordinal_suffix(17) == "th"
      assert EventDateFormatter.ordinal_suffix(18) == "th"
      assert EventDateFormatter.ordinal_suffix(19) == "th"
      assert EventDateFormatter.ordinal_suffix(20) == "th"
      assert EventDateFormatter.ordinal_suffix(21) == "st"
      assert EventDateFormatter.ordinal_suffix(22) == "nd"
      assert EventDateFormatter.ordinal_suffix(23) == "rd"
      assert EventDateFormatter.ordinal_suffix(24) == "th"
      assert EventDateFormatter.ordinal_suffix(25) == "th"
      assert EventDateFormatter.ordinal_suffix(26) == "th"
      assert EventDateFormatter.ordinal_suffix(27) == "th"
      assert EventDateFormatter.ordinal_suffix(28) == "th"
      assert EventDateFormatter.ordinal_suffix(29) == "th"
      assert EventDateFormatter.ordinal_suffix(30) == "th"
      assert EventDateFormatter.ordinal_suffix(31) == "st"
    end
  end

I am making the assumption there is never more than 31 days.

Thanks for the helpful library.

@darraghenright
Copy link

Hello there 👋

What's the latest with this PR? It's a couple of years old now, but if it's still a desired feature I'd be happy to do further work on it.

@tfantina
Copy link

tfantina commented Jan 9, 2024

I just came across this attempting to create a label for some graphs I'm making. I'd be happy to pick up the work on this rather than, effectively, rewriting it in my codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants