Problem/Motivation

One of the new fancy features of D8 UX is that when you select a Drupal.de.po translation file in import translations, it automatically pre-selects German (de). This is not the case if the file is called de.po. As this is the default file name when exporting Gettext PO files, this should be supported.

Forking from https://drupal.org/comment/8680903#comment-8680903

Proposed resolution

Detect the language on langcode.po files, as it does with project.langcode.po files.

Remaining tasks

Review the attached patch.

User interface changes

The selected language changes when the file is selected.

API changes

None.

CommentFileSizeAuthor
#9 2240555-9.patch940 bytespenyaskito
#6 2240555-6.patch787 bytespenyaskito
#1 2240555-1.patch941 bytespenyaskito
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
FileSize
2240555-1.patch941 bytes

Attached a patch, which modifies the regular expression so myproject.langcode.po and langcode.po are both supported.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Active » Needs review
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work 
+++ b/core/modules/locale/locale.bulk.js
@@ -14,9 +14,11 @@
+            // If there is a language which prefix is on the filename before the
+            // extension, pre-select it on the language input.
+            var matches = $(this).val().match(/(|\.)([\-\w]+)\.po/);

I think the English is not very clear here. We support two use cases. Either the filename is fully the language code or the filename ends with a language code.

I'm also not sure '\w' and '-' would match all characters possible in a language code?

Maybe we want the pattern as '/(ˆ|\.)([ˆ\.]+)\.po/ that is the start of the string or a dot followed by non-dots followed by .po, where we would take the non-dots?

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented

Some browsers (the modern ones) prepend c:\fakepath\ to the filename, see http://lists.w3.org/Archives/Public/public-whatwg-archive/2009Mar/0309.html

So we need a solution that matches:

/var/www/es.po
/var/www/es-ES.po
/var/www/drupal.es.po
/var/www/drupal.es-ES.po
c:\fakepath\es.po
c:\fakepath\es-ES.po
c:\fakepath\drupal.es.po
c:\fakepath\drupal.es-ES.po
es.po
es-ES.po
drupal.es.po
drupal.es-ES.po

Your proposed regexp doesn't match the Windows/Unix path ones.
Building from there I got to /([^.|)*(\.|\\|/)*([^.]+)\.po$/, but I think readability is worse. So I would stick with /(|\.)([\-\w]+)\.po/. Agree?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

I never used pipe with an empty component. Would (|a) match ba too? Because it is b followed by empty followed by a :)

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Needs work » Needs review
FileSize
2240555-6.patch787 bytes

No, (|a) doesn't match ba. I don't really know why it works, but it does.
Maybe we should stick to /([^.|)*(\.|\\|/)*([^.]+)\.po$/, which is more complex to parse on your head but easier to understand?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

([^.|)* seems to be not only hard to parse in head but also invalid? [ would need to be followed by ] at some point.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: -language-interface +language-ui

Fix tags.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Needs work » Needs review
FileSize
2240555-9.patch940 bytes

New attempt, using /([^.][\.]*)([\w-]+)\.po$/: whatever followed by a dot (or nothing), langcode, which can be words and hyphens (es, ca-ES, etc), and the .po extension.

This has worked with all my tests, but I don't have ATM any browser which treats upload files as UNIX paths.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Looks all good to me.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2240555-9.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review

9: 2240555-9.patch queued for re-testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Totally random fails.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Good one for behat... Committed/pushed to 8.x, thanks!

  • Commit 33d8ee8 on 8.x by catch: 
    Issue #2240555 by penyaskito: Preselect language when importing...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: -sprint

Superb, thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.