Piwigo icon indicating copy to clipboard operation
Piwigo copied to clipboard

international date formats

Open mistic100 opened this issue 5 years ago • 10 comments

On the photo property page there is a block with the following text : "Posted the 4 August 2016" It should be "Posted on August 4th, 2016"

mistic100 avatar Feb 06 '21 11:02 mistic100

Hi, I'm working on this issue and have a fix. However, as a new contributor I'm a little lost about the right procedure to do my PR.

  1. Is there a recommanded setup for the developper (PHP version, etc...)
  2. Is there a continuous integration or unitary test to be sure I will not break everything?
  3. If I have general questions about piwigo code, is this issue the best place or is there another place (IRC channel, slack, etc...) to ask?
  4. Is there a kind of roadmap where I should look up for most important issues to fix prior others?

For the time being, I have 2 questions:

  • is $user['language'] the best way to get user language?
  • to fix this issue, I've used the IntlDateFormatter class which requires the PHP intl extension. So, it means I'm adding a new dependency to the piwigo project. I wonder if I can decide it myself withour referring to "true" piwigo developers.

Thanks, Pol

cpol0 avatar Apr 18 '21 09:04 cpol0

However, as a new contributor I'm a little lost about the right procedure to do my PR.

The guide is on https://github.com/Piwigo/Piwigo/blob/master/docs/CONTRIBUTING.md but it may need more details if you didn't find it clear enough :-)

1. Is there a recommanded setup for the developper (PHP version, etc...)

Just compatible prerequisites, see https://piwigo.org/guides/install/requirements

2. Is there a continuous integration or unitary test to be sure I will not break everything?

No.

3. If I have general questions about piwigo code, is this issue the best place or is

there another place (IRC channel, slack, etc...) to ask?

For general question, go to https://piwigo.org/forum/viewforum.php?id=24

4. Is there a kind of roadmap where I should look up for most important issues to fix prior others?

On this issue tracker (Github), we have set milestones for issues. You can filter on the next milestone if you're looking for the highest priority issues. Next stable milestone is 11.5.0, next unstable milestone is 12.0.0beta1

For the time being, I have 2 questions:

* is `$user['language']` the best way to get user language?

Sure, yes.

* to fix this issue, I've used the `IntlDateFormatter` class which requires the

PHP intl extension. So, it means I'm adding a new dependency to the piwigo project. I wonder if I can decide it myself withour referring to "true" piwigo developers.

Piwigo already has a function format_date to deal with date format. I admit it's not great because "2021-04-18" because "Sunday 18 April 2021" instead of a more accurate "Sunday April 18th, 2021" as suggested by @mistic100

Did you try to work with the DateTime class? @mistic100 has started to use it in Piwigo code a while ago and we can safely rely on it now.

plegall avatar Apr 18 '21 12:04 plegall

Then the other "bug" is that in English/American (en_UK/en_GB/en_US) the value for language key Posted the %s is Posted the %s while it should be Posted on %s. We should not change the language key, just the language value, and this is a translation matter, not really a coding matter.

plegall avatar Apr 18 '21 12:04 plegall

Thanks for you reply! yeah, I read the contributing guide but effectively, there are not a lot of details inside and I wish to do as best as possible :)

  1. great, I'm currently on 7.4 and wish to know if I must also check with PHP5. So this is not the case, ouf!
  2. Ok!

And, in fact, the fix will be in the format_date. By the way, there is a TODO use IntlDateFormatter for proper i18n which give the solution inside the function :) For your last comment, so I did it the wrong way :-( I've effectively changed the key, but you're right this is not the right way. I will so change the english translation, it will be cleaner.

With my fix, I have for french:

Postée le 17 avril 2021(post info) dimanche 18 avril 2021 à 11:44 (comment info)

For english:

Posted on April 17, 2021 (post info) Sunday, April 18, 2021 at 11:44 AM (comment info)

Regarding my question about the TU, it was to check if it is possible to have another date format than ['day_name', 'day', 'month', 'year'] or [ 'day', 'month', 'year']. For example is it possible to have only year or month + year? It doesn't make sense but because I use predefined IntlDateFormatter constant, it's important to know.

polochon777 avatar Apr 18 '21 12:04 polochon777

What is "TU"?

Actually I wonder what the problem @mistic100 is mainly asking for fixing:

  1. changing posted the %s into posted on %s (very easy)
  2. changing 4 August 2016 into August 4th, 2016 (quite complicated)

plegall avatar Apr 18 '21 15:04 plegall

TU = "Test Unitaire" I guess :-)

I think both should be fixed, using i18n API is a good idea (given enough support)

mistic100 avatar Apr 18 '21 15:04 mistic100

I think both should be fixed, using i18n API is a good idea (given enough support)

The "given enough support" is very important here. On one side we have the month names already translated in Piwigo and we can safely rely on it. On the other side, we have to cross our fingers that the expected locales are available on the environment. For me, it's easy to pick the most appropriate solution.

plegall avatar Apr 18 '21 15:04 plegall

Yes sorry for the french, I did so much unitary tests in my previous life than "TU" is burned forever in my brain :) And for me, the issue is concerning the 2 points you mentionned @plegall : the "posted on" AND the date format. For the last point, I've always understood that intl is independant and manage every possible translations so in my mind this is very reliable. As I've said in the PR, I will try to hardcode some langages that are not in my setup (a custom docker setup with only french & english), we will see if the fix does the job or not.

cpol0 avatar Apr 18 '21 17:04 cpol0

question for @cpol0 : if the de_DE.utf8 locale is not installed on my server, will the date be correctly displayed in German?

plegall avatar Nov 18 '23 15:11 plegall

I will not bet on that. https://www.php.net/manual/en/intldateformatter.create.php is not very clear on this point, what happen if locale links to a locale not installed on the server. I've checked quickly the source code https://github.com/php/php-src/blob/master/ext/intl/dateformat/dateformat_create.cpp#L113C46-L113C46, again it's not obvious what will be returned (I didnt' check the code in Locale::createFromName .) So the behavior will probably be:

  • if you have some chance: the same as if you pass null, so the default locale
  • more probably: this is an error, so print will be set to null.

Probably, the best way is to test but I didn't have a piwigo setup anymore :/

polochon777 avatar Nov 26 '23 15:11 polochon777