As a followup to #567832-113: Transliteration in core and see also #567832-103: Transliteration in core:

Problem/Motivation

The core language overrides for specific character transliterations are currently using PHP files, while contrib modules have to use a hook for language-specific overrides. It would be better if they both used the same method, so code could be simplified.

Proposed resolution

Both the core and contrib language overrides would use YAML files.

Remaining tasks

a) Decide if this is a good idea or not.
b) If it is a good idea, change the code to use YAML files.

User interface changes

None.

API changes

- hook_transliteration_overrides_alter() would go away.
- The code in the PHPTransliteration class that reads language overrides would read them from YAML files instead of PHP files.

Original report by jhodgdon

The core language overrides for specific character transliterations are currently using PHP files, while contrib modules have to use a hook for language-specific overrides. It would probably be better if the core language overrides used the hook as well.

That would probably mean a vast simplification of the code that is currently checking for language overrides, since it would just override the hook instead of also reading data files.

Comments

gábor hojtsy’s picture

Issue tags: +D8MI

Tagging for multilingual for tracking.

sun’s picture

I disagree with this idea. The transliteration data is part of the Transliteration component itself, which is useful outside of Drupal.

Also, there is no reason for why a module that implements the hook would have to not use a file — a module can simply read in a file, identical to how the Transliteration component does it.

amateescu’s picture

Transferring my comment from #1842726-6: Transliteration component must not contain drupal_alter().

I disagree as well, these are not core (Drupal) specific overrides, they need to be available for anyone who wants to use this class.

That hook is useful for contrib or custom code that might have a very specific use case for overrides, but any improvements to the base overrides (these appear from time to time in the contrib Transliteration issue queue) need to placed in our base class.

jhodgdon’s picture

OK, I'm convinced. :)

Please convince @catch, who suggested this idea in the first place on the other issue (see comments referenced in this issue original statement).

amateescu’s picture

Assigned: Unassigned » catch
Status: Active » Needs review

Let's make sure he sees this then :)

catch’s picture

Assigned: catch » Unassigned

I never said hook, I suggested YAML files.

jhodgdon’s picture

Title: Transliteration: Make core language-specific character overrides use hook » Transliteration: Make language-specific character overrides use YAML files
Status: Needs review » Active

Ah, OK. I'll update the summary.

jhodgdon’s picture

We need to figure this out before we embark on #1842726: Transliteration component must not contain drupal_alter() I think. That issue is thinking about how to separate out the drupal-specific parts of transliteration into a separate class... but I think if we use YAML it is all Drupal-specific maybe? I am not sure...

sun’s picture

I don't think that changing the data files to YAML is a good idea.

1) The YAML parser is slow.

2) This adds a major dependency on the YAML parser to parse _all_ Unicode/UTF-8 correctly.

3) The YAML parser itself is a needless dependency. The Transliteration component can be used as a standalone component right now, without any further dependencies.

4) There's no evidence that adding or editing transliteration data in YAML files is any easier than adding/editing the PHP files.

jhodgdon’s picture

OK... let's step back a moment.

I think the main idea of this issue was that core and contrib modules have different ways of specifying language overrides: core uses these *.php files, and contrib uses a hook. I guess the questions are:

--- Questions ---

a) Is this inconsistency between how core and contrib specify language overrides actually a problem?

b) If so, is there a good/acceptable way we can get rid of the inconsistency?

--- Answers ---

a) Apparently catch thinks the inconsistency is a problem. It seems like the rest of us probably don't?

b) The two solutions that have been proposed for making core/contrib behave the same are:

1. Use a hook for both core & contrib - no one likes this idea, because it would mean we can't have a stand-alone Drupal-independent Component class that still offers language overrides.

2. Use YAML files for both core & contrib - this is not great (see #9) because it's slow and introduces a YAML parser dependency even to the Component, so you wouldn't be able to do any transliterations without YAML.

Are there any other possible solutions? In Drupal 7.x, the only way contrib could change core behavior was a hook, and not being steeped in Drupal 8 yet, that is what I used in the Transliteration class.

In Drupal 8, I guess we have more options for how contrib can change core behavior:
- hooks
- YAML/configuration
- plugins
- changing classes returned by the container
- Anything else?

Would any of these provide a good method for contrib to override language-specific transliterations, and also be good for the core Component class? Because it seems to me that an alter hook is a good semantic match to what is happening here, since we're just giving contrib the opportunity to change an array that is defined in core. And I don't think the mismatch between the way core happens to pick up its default array (from the PHP files) and the way contrib is allowed to alter them (an alter hook) is a huge problem.

Any other thoughts?

catch’s picture

fwiw I'm fine if the answer to this issue is 'no', I just found it a bit strange seeing the completely raw PHP files here.

I'm not sure YAML parser being slow is a good answer to this, how often do these have to be checked? And when they're checked does that block any other processes (I'd hope not).

Having them in .php isn't necessarily 'fast' - if they're rarely checked they're unlikely to be in the APC cache when parsed, then they'll be taking up space in there once they are. However they're pretty small files so it's not a big deal compared to other things.

sun’s picture

TBH, I'd prefer to mark this won't fix and leave it to hook implementors to decide on their own whichever format they want to use.

The example code in the hook documentation goes even one step further and suggests to use no separate/external file at all.

And I think that's much more in line with the reality that I predict we'll be facing:

- The hook can be used to override transliteration data decisions of the Transliteration component.

- However, any additions and corrections to transliteration data will end up as patches against Drupal core.

The contrib Transliteration module didn't have this hook in the first place. That aspect may be understood as a technical flaw, but it forced all users to improve the actual transliteration data that ships with the module.

And it's perfectly possible that we should clarify this in the alter hook documentation à la:

You can use this hook to apply custom transliteration data overrides, but if you have any additions or corrections for the transliteration data supplied by Drupal core, please contribute them to Drupal core instead, and only use this hook as a temporary override until core has been improved.

jhodgdon’s picture

+1 to #12.

I think the example in the hook documentation does provide another use case idea though -- maybe for a specific site or a specific country's use of a particular language, they just don't happen to like the Core defaults, so they override a few characters.

jhodgdon’s picture

Status: Active » Closed (won't fix)

I added something to the hook documentation in the patch for
#1858376: Provide tests and documentation for long Unicode characters
to address #12. I think we pretty much all agree that this issue is a "won't fix", so at least for now I'm closing it as such.

jhodgdon’s picture

Component: base system » transliteration system

Component update for posterity.

jhodgdon’s picture

Issue summary: View changes

update to explain YAML files are the idea not hook.

klonos’s picture

Issue summary: View changes
Parent issue: » #567832: Transliteration in core

...testing issue relationships ;)