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

Add basic Russian support #23

Closed
wants to merge 2 commits into from
Closed

Add basic Russian support #23

wants to merge 2 commits into from

Conversation

Forst
Copy link
Contributor

@Forst Forst commented Oct 28, 2015

This commit adds basic Russian support to DateTimeDifference.

However, fixes are required. The compound items are supposed to be different for "past" and "future" dates. For now they're translated as "past". Also, the "future" prefix (from_now) goes before the text:

second:
  past: "%count% секунда назад|%count% секунды назад|%count% секунд назад"
  future: "через %count% секунду|через %count% секунды|через %count% секунд"

The translation format is taken from Symfony/Component/Translation/PluralizationRules.php and has to be tested.

@norberttech
Copy link
Member

@Forst thanks! Could you also please add one test before I merge that? Just like here: https://github.com/coduo/php-humanizer/blob/master/spec/Coduo/PHPHumanizer/DateTimeSpec.php#L175

@Forst
Copy link
Contributor Author

Forst commented Oct 29, 2015

@norzechowich Added a unit test for precise DateTime humanization according to how other languages were done. However, like other languages, "month/months" and "week/weeks" were not tested. Should I add such test cases as well?

@norberttech
Copy link
Member

@Forst yes please, if this is not a problem for you of course.
Please also check why tests are failing.

@Forst
Copy link
Contributor Author

Forst commented Oct 29, 2015

@norzechovicz Will do. One thing is not entirely clear for me: will 35 days difference convert to "5 weeks" or "1 month 5/6 days"?

Also, should I add a test for separate items (i.e. not precise)?

The tests are failing because, as I said, from_now is a prefix in Russian, not a suffix. I can try to add support for prefixes, but it'll break existing translation pull requests.

@norberttech
Copy link
Member

Ah right, prefix. Mmm sounds quite similar to #26 do you think its the same problem?

@Forst
Copy link
Contributor Author

Forst commented Oct 29, 2015

#26 has exactly the same problem. I suggest we introduce past_prefix and past_suffix, same for from_now.

@orestes
Copy link

orestes commented Oct 29, 2015

I agree with @Forst. Introducing past_prefix, past_suffix, future_prefix and future_suffix would allow for more flexible translations.

@Forst
Copy link
Contributor Author

Forst commented Oct 29, 2015

I think it's possible to go further. Ideally, there should be no need to translate seconds, hours, months and so on twice: once for simple and once for precise dates. Instead, there should be common translations for both cases:

units:
    seconds: 'second|seconds'
    minutes: 'minute|minutes'
    ## snip ##
    years: 'year|years'

special:
    now_past: 'just now'
    now_future: 'just now'

prefixes:
    past: ''
    future: ''

suffixes:
    past: 'ago'
    future: 'from now'

@norberttech
Copy link
Member

@Forst that make sense, could you prepare a PR for that?

@Forst
Copy link
Contributor Author

Forst commented Oct 29, 2015

@norzechowicz Will try to, I might need some help later on though. I'm closing this PR, will include Russian in the new request.

@Forst Forst closed this Oct 29, 2015
@norberttech
Copy link
Member

@Forst thanks! Contact me if you would need any help.

@sam002
Copy link
Contributor

sam002 commented Oct 30, 2015

@Forst Hi! Please check my changes for prefix support: #41
It as your design?

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.

4 participants