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

Bug: Time::humanize() does not work with ar locale #4708

Closed
perspolise opened this issue May 19, 2021 · 6 comments · Fixed by #6120
Closed

Bug: Time::humanize() does not work with ar locale #4708

perspolise opened this issue May 19, 2021 · 6 comments · Fixed by #6120
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@perspolise
Copy link

perspolise commented May 19, 2021

I set arabic language in app config file for defualtLocale like this:

public $defaultLocale = 'ar';

Now, I work with humanize time function like this:

$time = CodeIgniter\I18n\Time::parse($date, 'America/Chicago');
return $time->humanize(); 

Now in action I get this error:


DateTime::__construct(): Failed to parse time string (٢٠٢١-٠٥-١٩ ٠١:٤٧:٢٨) at position 0 (�): Unexpected character

SYSTEMPATH\I18n\Time.php at line 1150

public function humanize()
1149     {
1150         $now  = IntlCalendar::fromDateTime(Time::now($this->timezone)->toDateTimeString());

I think the DateTime function have problem with arabic number. because when I set en for default locale, humanize function work truly.

@lonnieezell @MGatner

@MGatner
Copy link
Member

MGatner commented May 20, 2021

No need to ping, I read every issue and respond according to my ability and time.

I don't have much experience with Unicode string handling. Hopefully someone familiar will step in.

@paulbalandan
Copy link
Member

I cannot reproduce that this works using the default locale en. Even this simple script fails.

$date = '٠١:٤٧:٢٨';
$time = (new DateTime())->modify($date);
var_dump($time);

Unfortunately, this is not a CI4 bug. This is an inherent limitation of the DateTime parser used by PHP. It can only understand Latin input as described here.

@perspolise
Copy link
Author

perspolise commented May 22, 2021

@paulbalandan Sure, You right. But if we need to a multilingual system, humanize not work truly(for en it's ok, for others is not ok). I think CI4 doesn't need to convert/translate numbers.

@paulbalandan
Copy link
Member

Yes, I understand what you want. But this issue is not limited to Time's humanize() function. The issue lies internally in the PHP core. Since PHP's own DateTime doesn't understand Arabic dates, naturally it follows that CI4's Time (which is an extension of DateTime) will also fail with Arabic dates. As you can see, even DateTime::modify() fails parsing the date so this is outside CI4.

What I can see as a potential solution is to have first a converter to/from Arabic dates. Something like strtr replacements. After conversion, pass it to DateTime/Time. However, I don't see that converter fit into the framework as we would need to also cater other languages with non-Latin datetimes.

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jun 14, 2022
@kenjis
Copy link
Member

kenjis commented Jun 14, 2022

I've confirmed this bug.

diff --git a/app/Config/App.php b/app/Config/App.php
index c6c716822..b697a23c5 100644
--- a/app/Config/App.php
+++ b/app/Config/App.php
@@ -70,7 +70,7 @@ class App extends BaseConfig
      *
      * @var string
      */
-    public $defaultLocale = 'en';
+    public $defaultLocale = 'ar';
 
     /**
      * --------------------------------------------------------------------------
@@ -97,7 +97,7 @@ class App extends BaseConfig
      *
      * @var string[]
      */
-    public $supportedLocales = ['en'];
+    public $supportedLocales = ['ar', 'en'];
 
     /**
      * --------------------------------------------------------------------------
diff --git a/app/Controllers/Home.php b/app/Controllers/Home.php
index 7f867e95f..ff48fb5f2 100644
--- a/app/Controllers/Home.php
+++ b/app/Controllers/Home.php
@@ -6,6 +6,8 @@ class Home extends BaseController
 {
     public function index()
     {
-        return view('welcome_message');
+        $date = '2022-06-06';
+        $time = \CodeIgniter\I18n\Time::parse($date, 'America/Chicago');
+        return $time->humanize();
     }
 }
$ php public/index.php 


[Exception]

DateTime::__construct(): Failed to parse time string (٢٠٢٢-٠٦-١٤ ٠٠:٢٨:١٠) at position 0 (?): Unexpected character

at SYSTEMPATH/I18n/Time.php:1008

Backtrace:
  1    [internal function]
       DateTime()->__construct('٢٠٢٢-٠٦-١٤ ٠٠:٢٨:١٠')

  2    SYSTEMPATH/I18n/Time.php:1008
       IntlCalendar::fromDateTime('٢٠٢٢-٠٦-١٤ ٠٠:٢٨:١٠')

  3    APPPATH/Controllers/Home.php:11
       CodeIgniter\I18n\Time()->humanize()

  4    SYSTEMPATH/CodeIgniter.php:898
       App\Controllers\Home()->index()

  5    SYSTEMPATH/CodeIgniter.php:468
       CodeIgniter\CodeIgniter()->runController(Object(App\Controllers\Home))

  6    SYSTEMPATH/CodeIgniter.php:351
       CodeIgniter\CodeIgniter()->handleRequest(null, Object(Config\Cache), false)

  7    FCPATH/index.php:55
       CodeIgniter\CodeIgniter()->run()

@kenjis kenjis reopened this Jun 14, 2022
kenjis added a commit to kenjis/CodeIgniter4 that referenced this issue Jun 14, 2022
kenjis added a commit to kenjis/CodeIgniter4 that referenced this issue Jun 14, 2022
@kenjis
Copy link
Member

kenjis commented Jun 14, 2022

@perspolise
I've fixed this bug. See #6120

But the output is like this:

١ week ago

Is this correct?

(Added)
https://www.saitak.com/Number/
1 is ۱. Okay.

@kenjis kenjis changed the title DateTime::__construct(): Failed to parse time string () at position 0 (�): Unexpected character Bug: DateTime::__construct(): Failed to parse time string () at position 0 (�): Unexpected character Jun 14, 2022
@kenjis kenjis changed the title Bug: DateTime::__construct(): Failed to parse time string () at position 0 (�): Unexpected character Bug: Time::humanize() does not work with ar locale Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants