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 possibility to use translated short notations #229

Merged
merged 1 commit into from
Mar 12, 2017

Conversation

Boschman
Copy link
Contributor

Fix for #226

  • use short notation translations if available
  • add short notation translations in German and Dutch
  • update tests

@@ -243,7 +243,16 @@ public function format($format)

// Short notations.
if (in_array($character, ['D', 'M'])) {
$translated = mb_substr($translated, 0, 3);
$toTranslate = strtolower($original);
$sortTranslated = $lang->trans($toTranslate);

Choose a reason for hiding this comment

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

s/sort/short/

Copy link

@danhunsaker danhunsaker left a comment

Choose a reason for hiding this comment

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

Minor quibble that the variable $sortTranslated should probably be $shortTranslated instead. Otherwise, looks good to me.

@Boschman
Copy link
Contributor Author

Thanks! I just fixed the typo.

@Boschman
Copy link
Contributor Author

Boschman commented Jan 7, 2017

@jenssegers, could you please let me know what you think of this pull request? Would you be able to merge the request?

Please let me know if you want me to change anything.

@@ -243,7 +243,16 @@ public function format($format)

// Short notations.
if (in_array($character, ['D', 'M'])) {
$translated = mb_substr($translated, 0, 3);
$toTranslate = strtolower($original);
Copy link
Contributor

Choose a reason for hiding this comment

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

mb version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm more than willing to change this to mb_strtolower, but the original source code uses strtolower as well: https://github.com/jenssegers/date/blob/master/src/Date.php#L241

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just changed strtolower to mb_strtolower.

* use short notation translations if available
* add short notation translations in German and Dutch
* update tests
@Boschman
Copy link
Contributor Author

Is there anything more I can do to get this pull request accepted? Please let me know, @jenssegers.

@Rjs37
Copy link

Rjs37 commented Mar 4, 2017

@jenssegers Would love to see this pull request accepted. So we can start providing some abbreviated translations too.

Apparently what's appearing for an abbreviated Monday in Arabic translates as "However" in Google Translate lol.

@jenssegers jenssegers merged commit 0b31adf into jenssegers:master Mar 12, 2017
@Boschman Boschman deleted the short-notation-translations branch August 30, 2018 20:35
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.

None yet

5 participants