LaravelRUS / localized-carbon

A localizable version of Carbon
138 stars 62 forks source link

Add info about genitive months #39

Closed vladshcherbin closed 9 years ago

vladshcherbin commented 9 years ago

Hi, now there is not much info about that feature.

According to the docs, I have to use %f, but, unfortunately, I have to use %%f to make it work. This is not straightforward, especially for a novice.

So, I suggest correcting the docs with %%f and probably adding some example, smth like this:

// Output in RU: 16 декабря 2014, 18:11
LocalizedCarbon::instance($post->created_at)->formatLocalized('%d %%f %Y, %H:%M');
Amegatron commented 9 years ago

Thats strange, I just tested it again, and just %f works fine for me.

Route::get('/test', function() {
    $date = \Laravelrus\LocalizedCarbon\LocalizedCarbon::create(2014, 6, 12);
    return $date->formatLocalized('%f');
});

Output is июня.

vladshcherbin commented 9 years ago

@Amegatron probably, you are using windows. check the Carbon format method, there is a preg_replace function there for win users.

After this method, not on windows, I just get the f letter, your example also shows the f letter.

Latest version of 4.2 laravel, latest version of this package, mac os.

Amegatron commented 9 years ago

I'm using homestead - so this is not under Windows. I checked the method used to determine Windows in original formatLocalized:

strtoupper(substr(PHP_OS, 0, 3))

It returns LIN.

I'm also having latest versions of Laravel and Carbon.

I suppose the issue occurs only under Mac. I'll think about better solution of it, but I need someone else to approve it on Mac.

vladshcherbin commented 9 years ago

@Amegatron yes, that's strange

the native Carbon formatLocalized() function returns

return strftime($format, $this->timestamp);

With %f it returns just the f letter. Idk why this differs. I guess, that function works not the same on different php/os versions.

My php version is 5.5.10

vladshcherbin commented 9 years ago

@Amegatron ok, I tested it. In fact, there is no %f key in strftime function.

I suggest using the standard %B key for that.

I also tested the solution:

public function formatLocalized($format = self::COOKIE)
{
    $result = parent::formatLocalized($format);

    if (strpos($format, '%B') !== false)
    {
        $month = parent::format("F");
        $localizedMonth = \Lang::get("localized-carbon::months." . strtolower($month));
        $result = str_replace($month, $localizedMonth, $result);
    }

    return $result;
}

or even a shorter version

public function formatLocalized($format = self::COOKIE)
{
    if (strpos($format, '%B') !== false)
    {
        $month = strtolower(parent::format("F"));
        $localizedMonth = \Lang::get("localized-carbon::months." . $month);
        $format = str_replace('%B', $localizedMonth, $format);
    }

    return parent::formatLocalized($format);
}

Also, consider changing lang files from январь to Январь - that is the default behavior, not the lowercase one.

Hope, this will help. Thanks for the package ;)

Amegatron commented 9 years ago

That is the case, %f is specially the key that is not used by default strftime to not conflict with default behaviour. And also thats why I don't want to replace %B because what if someone wants the default behaviour? I think that would not be a good solution. Considering strftime, I also now think that the reason is the difference between implementations of this function under different systems.

Amegatron commented 9 years ago

@VladShcherbin Please check out my possible fix: https://github.com/LaravelRUS/localized-carbon/commit/e00eb796a03d00ca3e306c6c833ee44eb16e2caa. I first replace %f and only then pass the string to parent::formatLocalized.

vladshcherbin commented 9 years ago

@Amegatron yes, that is definitely working.

you can shorten it by

public function formatLocalized($format = self::COOKIE)
{
    if (strpos($format, '%f') !== false) {
        $month = strtolower(parent::format("F"));
        $localizedMonth = \Lang::get("localized-carbon::months." . $month);
        $format = str_replace('%f', $localizedMonth, $format);
    }

    return parent::formatLocalized($format);
}

All in all, please consider changing to the %B key & first letter uppercase, this is the expected behavior, there is no point in reinventing the wheel.

And if someone would want the default behaviour, the standard Carbon class should be used instead.

Thanks ;)

Amegatron commented 9 years ago

I'll think about switching to %B, but I'm not going to make first letter uppercase for Russian localization. First uppercase character is the standard in USA and England and maybe somewhere else, but not in Russia, we have lower-case characters, we do not write "16 Января 2014", but "16 января 2014". Even standart %B behaviour uses lower-cased months.

Anyway, thanks for pointing out the issue about %%f under Mac :)

Amegatron commented 9 years ago

@VladShcherbin sorry, I meant %b - abbreviated month, wich is in genitive for russian, e.g. "мая".

vladshcherbin commented 9 years ago

@Amegatron yes, i checked the default %B in russian, the result is lowercased.

So, if the %f will be switched to the default %B it would be super cool. Thanks.