Automattic / wp-cldr

Use CLDR localization data in WordPress
GNU General Public License v2.0
24 stars 6 forks source link

Better handling of missing JSON files #72

Closed akirk closed 5 years ago

akirk commented 8 years ago

In light of potentially downloading the CLDR files separately, missing files should be handled better.

Currently the WP CLDR demo page looks like this:

missing-json

Probably it would be useful to display a notice in WP-Admin about the missing files.

akirk commented 8 years ago

Related #37

akirk commented 8 years ago

From a coding standpoint I'd suggest to return true or false in initialize_locale_bucket() and introduce a $this->initalized class variable and check for that before accessing the localized array.

akirk commented 8 years ago

As a continuation, you could introduce a class CLDR_Exception extends Exception {} and use throw new CLDR_Exception( 'missing JSON file' ) in the initializing.

stuwest commented 8 years ago

This makes sense. I see a few separate issues:

  1. Improved handling of the English fallback. Currently, there isn't any visibility of the fallback outside of initialize_locale_bucket(). Having that method return true or false makes sense. I don't think we'd need a $this->initialized class variable since we can just check isset( $this->localized... ). Related, because of the way English fallbacks are handled a locale may be "initialized" and in memory but just have English data. It would be better to avoid "initializing" those locales. I'll do that and move the fallback logic into get_locale_bucket().
  2. Messaging around missing locale files. Yes it would be good to have the plugin settings page check for the currently installed languages (get_available_languages()?) and then display a message if any don't map to an installed CLDR JSON file.
  3. better exception handling of missing files so we don't have that mess of notices and warnings.

@akirk, how did you get to the screenshot above? did you have to delete one of the CLDR files or did you get that with a fresh install of the plugin?

akirk commented 8 years ago

I renamed the json directory. I know it's not quite the fair thing to do but it was intended as preparation for the separate JSON downloading.

stuwest commented 8 years ago

OK @akirk I just added another commit https://github.com/Automattic/wp-cldr/commit/102ee31fdc39c8ffa49b02d3dfde256ab529b12a to PR #76. I think that PR now addresses this issue. The diff is a bit messy but it:

something is off with caching so I've disabled it for now I'll figure that out tomorrow.

Plugin page when selecting a locale without JSON files:

screen shot 2016-03-06 at 11 29 18 pm

Plugin page when entire JSON directory is gone:

screen shot 2016-03-06 at 11 28 39 pm
stuwest commented 5 years ago

closing as this as https://github.com/Automattic/wp-cldr/pull/76 was merged