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

Post age has no unit #539

Closed
axelrechenberg opened this issue Jun 11, 2023 · 10 comments
Closed

Post age has no unit #539

axelrechenberg opened this issue Jun 11, 2023 · 10 comments
Labels
4 - medium/high priority Issue needs to be fixed soon, does not break core functionality bug Something isn't working

Comments

@axelrechenberg
Copy link

** Jerboa Version **
0.0.33

Describe the bug
Post age has no unit - shows just a number without an indication whether that's seconds, minutes, hours etc.

To Reproduce
Happens every single time if my phone's language is set to Polish.

Screenshot_2023-06-11-21-48-12-32_a10f969cf1c4adf2c503042b459b8476

@axelrechenberg axelrechenberg added the bug Something isn't working label Jun 11, 2023
@gressen
Copy link

gressen commented Jun 11, 2023

Screenshot_20230611_220109_Jerboa for Lemmy
I'm experiencing the same bug, phone set to Polish as well.

@sensiblepuffin
Copy link

sensiblepuffin commented Jun 12, 2023

I have to go to sleep, but I figured I'd share what I found -

I tried 5 languages (English, Spanish, Polish, Korean, and Danish) and Polish was the only one that triggered this issue.

What's interesting is that, of these languages, Polish is the only locale for which override is defined in org.ocpsoft.prettytime.impl.ResourcesTimeFormat:L112:

return override == null ? super.format(duration) : override.format(duration);

For Polish, this override takes us to org.ocpsoft.prettytime.i18n.Resources_pl:L48, where only the number is returned:

public String format(Duration duration)
{
  long quantity = duration.getQuantityRounded(tolerance);
  return String.valueOf(quantity);
}

as opposed to org.ocpsoft.prettytime.format.SimpleTimeFormat:L95, which is where we go when there is no override above.

private String format(final Duration duration, final boolean round)
{
  String sign = getSign(duration);
  String unit = getGramaticallyCorrectName(duration, round);
  long quantity = getQuantity(duration, round);
  
  return applyPattern(sign, unit, quantity);
}

Update: Okay, I got curious and tried Russian. It, too, has an override there, and it too has missing time units. So format() behaves differently for languages with override ResourceBundles, and from what I can see it will never return the time unit.

Double update: If you look at the Resources_*.java files for languages that have overrides vs. no overrides, you see that the ones that work properly (no override) are mostly just a big 2D array with mapped out values. I grepped in that directory and it seems like there should be 4 languages that will have this issue: Kazakh, Polish, Russian, and Slavic.

@twizmwazin twizmwazin added the 4 - medium/high priority Issue needs to be fixed soon, does not break core functionality label Jun 14, 2023
@lbenedetto
Copy link
Contributor

I've filed a bug report with PrettyTime
ocpsoft/prettytime#259

dessalines added a commit that referenced this issue Jun 19, 2023
A bug in PrettyTime causes four languages to not get any time units when formatted. We can work around this by detecting an invalid prettyDate (rather than hardcoding a list of broken languages) and switching to English formatting instead.

ocpsoft/prettytime#259

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
@dawid2193487
Copy link

dawid2193487 commented Jun 21, 2023

This might be happening because Polish has different plurals for different numbers.

1 sekunda, 2..4 sekundy, 5.. sekund

@Snow4DV
Copy link
Contributor

Snow4DV commented Jun 21, 2023

This might be happening because Polish has different plurals for different numbers.

1 sekunda, 2..4 sekundy, 5.. sekund

It is already implemented for polish here:
https://github.com/ocpsoft/prettytime/blob/master/core/src/main/java/org/ocpsoft/prettytime/i18n/Resources_pl.java
same thing for russian:
https://github.com/ocpsoft/prettytime/blob/master/core/src/main/java/org/ocpsoft/prettytime/i18n/Resources_ru.java

There's probably a bug hiding somewhere

upd: Oh, ok, now i don't get why it was implemented but still returns simple String.valueOf in format override. Still looking into it

@lbenedetto
Copy link
Contributor

sensiblepuffin already explained what's wrong. It's because those four languages override TimeFormat::format and just return back the number passed in without attempting any logic. The implementation doesn't work because it doesn't exist. Look at what other languages, like Japanese do in their override to see how it should look.

@Snow4DV
Copy link
Contributor

Snow4DV commented Jun 21, 2023

sensiblepuffin already explained what's wrong. It's because those four languages override TimeFormat::format and just return back the number passed in without attempting any logic. The implementation doesn't work because it doesn't exist. Look at what other languages, like Japanese do in their override to see how it should look.

I see but i'm more curious how did it happen in the first place. There's issue that shows it working correctly in the newpipe some time ago: ocpsoft/prettytime#182 and nothing was changed in the code really since then in Resources_ru

@Snow4DV
Copy link
Contributor

Snow4DV commented Jun 21, 2023

Ok i looked into it and i discovered that while "formatDuration" is not implemented correctly in russian (and probably other languages) - format that basically return "1 hour ago" and etc works fine - that's what they used in NewPipe instead of "format" method. This unit test of library works totally fine:

public void testMinutesAgo3() throws Exception
   {
      PrettyTime t = new PrettyTime(new Date(1000 * 60 * 12), locale);
      assertEquals("12 минут назад", t.formatUnrounded(new Date(0)));
      assertEquals("12 минут назад", t.format(new Date(0)));
   }

   @Test
   public void testHoursAgo1() throws Exception
   {
      PrettyTime t = new PrettyTime(new Date(1000 * 60 * 60), locale);
      assertEquals("1 час назад", t.formatUnrounded(new Date(0)));
      assertEquals("1 час назад", t.format(new Date(0)));
   }

So just using "format" for these languages might be a better workaround

@Snow4DV
Copy link
Contributor

Snow4DV commented Jun 22, 2023

I solved this issue for ru locale in this pull request: ocpsoft/prettytime#260
Apparently it was done because there are some cases in russian that were impossible to support in current architecture without a little bit of hacky solution (but it is pretty much fine here - not a big deal) - while in english "1 minute " and "1 minute ago" are barely the same thing in russian it is "1 минута" and "1 минуту назад"

Methods "format*" and "decorate*" were behaving incorrectly with old implementation: "format*" was used to get string value of date and "decorate*" was used to do the rest of the stuff. Correct way is to implement "format*" to return something like "12 days" and "decorate*" to add " ago" or "in " suffix/prefix.

@dessalines
Copy link
Member

This doesn't seem to be an issue anymore.

@dessalines dessalines closed this as not planned Won't fix, can't repro, duplicate, stale Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - medium/high priority Issue needs to be fixed soon, does not break core functionality bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants