checkdomain / Holiday

Check whether a date is a holiday
www.checkdomain.de
MIT License
49 stars 27 forks source link

I18n Support #18

Closed wandersonwhcr closed 7 years ago

wandersonwhcr commented 7 years ago

Hello!

I just found your repository after thinking about a class that I developed 5 years ago with Zend_Date and holidays.

Reading your code, I just realized that you don't have a I18n support. So, if I code...

$util->isHoliday('PT', '25/12/2017');
// Exception: DateTime::__construct(): Failed to parse time string (25/12/2017) at position 0 (2): Unexpected character

I want to add this new feature. Do you have any suggestion before I begin?

After that, I'll add a new provider for Brazilian holidays.

Thank you!

wandersonwhcr commented 7 years ago

This new feature will use IntlDateFormatter to parse i18n dates.

Should I add a direct dependency in composer.json for ext-intl?

wandersonwhcr commented 7 years ago

Another problem is the locale name, with underscores (en_EN, pt_BR), breaking some PSRs in provider classes.

wandersonwhcr commented 7 years ago

Example:

diff --git a/lib/Checkdomain/Holiday/Util.php b/lib/Checkdomain/Holiday/Util.php
index e7b2959..62dae27 100644
--- a/lib/Checkdomain/Holiday/Util.php
+++ b/lib/Checkdomain/Holiday/Util.php
@@ -81,10 +81,11 @@ class Util
      */
     public function getHoliday($iso, $date = 'now', $state = null)
     {
-        $iso = $this->getIsoCode($iso);
-        $date = $this->getDateTime($date);
-
+        $iso      = $this->getIsoCode($iso);
         $provider = $this->getProvider($iso);
+
+        $date = $this->getDateTime($provider->parse($date));
+
         $holiday = $provider->getHolidayByDate($date, $state);

         return $holiday;
BenjaminPaap commented 7 years ago

I think your addition could be useful to some users but I would love to see a pull request that does not explicitly depend on ext-intl. It could be a suggestion in our composer.json and only be used if available.

See https://getcomposer.org/doc/04-schema.md#suggest

That said, it would be great if you could provide a pull request for this functionality.

wandersonwhcr commented 7 years ago

I agree. I'll work in a solution this weekend.

Ty!

wandersonwhcr commented 7 years ago

Hallo! Sorry for delay.

Thinking about the problem, I started to search why an input with dots works in PHP and I found why: it's a language internal feature.

https://github.com/php/php-src/blob/294f6168ed6a6ed3b4e536c11f83cc56687580ac/ext/date/lib/parse_date.re#L902-L918

The lexer function pointeddate4 have pattern /^day[.\t-]month[.-]year4$/, where day, month and year4 are sub-patterns, not described here.

This shows that day and month can be separated with dot, tab or dash; month and year4 can be separated with dot or dash.

So, we can create a DateTime object with this weird input:

<?php

var_dump(new DateTime("31\t12-2017"));
/*
object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2017-12-31 00:00:00.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(17) "America/Sao_Paulo"
}
 */

Now, I understand why DateTime separated with dots works and separated with slashes doesn't work.

Crazy, huh?

wandersonwhcr commented 7 years ago

IMHO, we must change the README.md example and tests, using ISO-8601 inputs instead of date string separated with dots, making docs internationalized. It will not make new readers confused.

wandersonwhcr commented 7 years ago

TY!