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

DateTime Humanizer : returns hours, days, months, but not weeks #765

Open
tranb3r opened this issue Dec 3, 2018 · 4 comments · May be fixed by #807
Open

DateTime Humanizer : returns hours, days, months, but not weeks #765

tranb3r opened this issue Dec 3, 2018 · 4 comments · May be fixed by #807

Comments

@tranb3r
Copy link

tranb3r commented Dec 3, 2018

Why does DateTime Humanizer return a number of hours, days, months, but never return a number of weeks ?
It does not seems consistent with TimeSpan Humanizer.

Example :
TimeSpan.FromDays(7).Humanize() => "1 week"
DateTime.UtcNow.AddDays(7).Humanize() => "7 days from now" (instead of "1 week from now")

@AKTheKnight
Copy link
Contributor

AKTheKnight commented Mar 24, 2019

@onovotny (sorry for the tag, let me know if I should stop tagging you), I think I'm going to need your input on this.

The method for choosing which unit is returned is calculated in DefaultHumanize() within DateTimeHumanizeAlgorithms.cs here:

public static string DefaultHumanize(DateTime input, DateTime comparisonBase, CultureInfo culture)

Currently the parts of the algorithm we are interested in work as described below (The third line is the bit we could change):

If it is less than 24 hours, return in hours.
If it is less than 48 hours, return in day.
If it is less than 28 days, return in days.
If it is more than 28 days, return in months.

The less than 28 days has the following code:

if (ts.TotalDays < 28)
{
    return formatter.DateHumanize(TimeUnit.Day, tense, ts.Days);
}

We could fix this issue by changing that code to look like the below to return Weeks for the area between 6 days and 28 days (exclusive):

if (ts.TotalDays < 7)
{
    return formatter.DateHumanize(TimeUnit.Day, tense, ts.Days);
}

if (ts.TotalDays < 28)
{
    return formatter.DateHumanize(TimeUnit.Week, tense, ts.Days);
}

This would then return the expected result of "7 days from now".

BUT, if this code is changed now we get a lovely little error of The resource object with key DateHumanize_MultipleWeeksFromNow' was not found

This is because there is no resource for "Weeks from now".

We could add one, but all the other localized languages would be missing this (I'm not sure what happens if it isn't there for a language).

How do you want to go forward with this (@onovotny). Is adding a new resource something you would want to wait until vNext for? As far as I can see no logic is really changed, we just return weeks instead of days for any values between 6 and 28 days (exclusive).

Alex

@clairernovotny
Copy link
Member

We can add a new resource in english, and solicit help from the community to fill in the blank.

@tranb3r
Copy link
Author

tranb3r commented Dec 2, 2019

It looks like a fix has been done a long time ago, but the PR has never been merged.
What exactly remains to be done ? How can I help ?

@chriszuercher
Copy link

I would also be intrested in this change. Is there any plan to change it?
Btw. I could provide the German and French translations if needed.

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 a pull request may close this issue.

4 participants