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

fix: formatDistance for Basque (eu) #3749

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

eillarra
Copy link

As a reference you can take a look at moment.js, which was updated years ago by me: https://github.com/moment/moment/blob/develop/locale/eu.js

For example, "en {result}" was still in Spanish...

eillarra and others added 2 commits March 29, 2024 22:49
As a reference you can take a look at moment.js, which was updated years ago by me: https://github.com/moment/moment/blob/develop/locale/eu.js

For example, "en {result}" was still in Spanish...
Copy link
Member

@fturmel fturmel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't speak the language, but the changes look good to me!

@fturmel
Copy link
Member

fturmel commented Mar 31, 2024

@eillarra can you confirm that there are no unintended behavior changes in the updated snapshot? d97aa76

@eillarra
Copy link
Author

eillarra commented Apr 1, 2024

@fturmel snapshot looks good. There is only one thing that it is not perfect, but I don't think it is possible with the current implementation:

6 urte gutxi gorabehera barru should be 6 urte barru gutxi gorabehera

But suffix is added after result is calculated, and the 'about' (gutxi gorabehera) is added beforehand to the result, so I guess it needs to stay like this. Thanks!

@eillarra
Copy link
Author

eillarra commented Apr 1, 2024

Keep this on hold for a moment... I only checked formatDistance, but I will check the other files too so the revision is complete. There might be a couple of small extra issues that can be solved.

@fturmel
Copy link
Member

fturmel commented Apr 1, 2024

Great! Here's the unicode CLDR reference if needed: https://www.unicode.org/cldr/charts/44/summary/eu.html

If you change anything else, you can update the snapshot markdown file yourself by running npm run locale-snapshots.

I'm sure it's possible to reshape the formatDistance function to support the "about" form you're describing. How does this look?

if (options?.addSuffix) {
  if (options.comparison && options.comparison > 0) {
    if (token.startsWith("about")) {
      return result.replace("gutxi", "barru gutxi");
    }
    return result + " barru";
  } else {
    return "duela " + result;
  }
}

@fturmel fturmel marked this pull request as draft April 1, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants