Round-up/down time units

[wiest]

I find it weird, that a difference of "almost 7 full days" is formatted as "7 days ago", whereas a difference of "a little more than 7 full days" if formatted as "a week ago".
Seeing items beside each other makes a user wonder what's going on.

I hope the example describes the issue. Probably this issue exists for any time units, where the quantity is just below 0, the next lower time unit is used, and when it reaches 1, the higher time unit is used.

I would suggest to round the quantity to the next aligned value of the lower time unit (days) before sending it to the higher time unit (week) for a check. Not sure how this can be done in the code.

[lincolnthree]

Hmmm, I tend to agree with that. I'm not exactly sure what the right approach would be to correct this behavior, but it's worth a look.

[wiest]

Right now, I'm wrapping the PrettyTime usage in a method that rounds the time difference to full days if the difference is more than 24h. This solves the 7 days vs. one week issue, which in my case is the only one I care about, but it's not nice obviously.

In pseudo code I could imagine something like:

var maxTimeUnit = findMaxTimeUnit(timeDiff)
var higherTimeUnit = findNextHigherTimeUnit(maxTimeUnit)
// Round to full units of the max time unit (e.g. days) and check if it now is a full unit of the next higher
var roundedTimeDiff = roundTimeDiffToUnit(timeDiff, maxTimeUnit)
if (getQuantity(higherTimeUnit, roundedTimeDiff) >= 1)
useTimeUnit(higherTimeUnit)
else
useTimeUnit(maxTimeUnit)

[lincolnthree]

Actually, I think the simplest fix is to configure the Day timeUnit in the initTimeUnits method here:

https://github.com/ocpsoft/prettytime/blob/master/core/src/main/java/org/ocpsoft/prettytime/PrettyTime.java#L696

Day day = new Day();
day.setMaxQuantity(6);
units.add(day);

I think this should have the desired effect, but it's possible there is a more precise solution.

[lincolnthree]

(Also, you can try updating the unit yourself locally (they are configurable via the PrettyTime API) if you want to get this working before I can "fix" it here and make a release.)

[lincolnthree]

Did you have any luck configuring the unit?

[wiest]

Actually, I already fixed this by aligning the duration to full days, which worked in my case. But your suggestion makes a lot more sense of course.

[sboukortt]

Would it be better with - 1 appended to this line?

[lincolnthree]

I'm not really sure what that would do? What's your line of reasoning for that change?

[sboukortt]

If I understand correctly, that line computes the default value for maxQuantity, but for Day, for example, unless I am mistaken, that would set it to 7 instead of 6 as in your fix above.

Is there anything that I got wrong?

[rococode]

I think this is related to an issue I had:

A time a little less than 24 full hours was listed as "24 hours ago" rather than "1 day ago".

But when I changed the Hours unit to have a maxQuantity of 23, I got a null error, presumably from the duration estimator not knowing how to fill in the time? The null was specifically on this line of PrettyTime:

TimeFormat format = this.getFormat(duration.getUnit());

[lincolnthree]

Issue cleanup Thanks for filing this issue. We are cleaning old issues from the repository. If the issue is still reproducible, please re-open with a test case to reproduce the behavior.

[sboukortt]

I believe that my script from #124 still reproduces the issue.