Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for non prefixed language files #19353

Merged
merged 6 commits into from Jan 21, 2019

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Jan 11, 2018

When I tried to use the GitHub integration of Crowdin, I faced the issue that Crowdin can't rename part of the translated filename. Eg it can't rename the source file en-GB.com_weblinks.ini to de-DE.com_weblinks.ini. It can however change the foldername easily, eg /language/en-GB/ can be changed to /language/de-DE/.

That made me question the need for the language prefix.
Since we already have the language specific in the folderstructure, there is no technical reason to add it also to the filename.

Summary of Changes

This PR changes the JLanguage logic so it first tries to load the file with prefix and if that fails it tries again with a non-prefixed filename.
This works for all files except the en-GB.ini (.ini doesn't make much sense) one and the ones in the override folder (we need the prefix here to differentiate the files).

This PR also removes some fallback code that tries to load the file from the default (en-GB) language. However since a few years we already do the same thing before even trying to load the real file. So those fallback strings are already loaded and that code did just load the same strings again.

Testing Instructions

  • Try renaming eg the language files from com_banners from de-DE.com_banners.ini to just com_banners.ini and see if the translation still works.
  • Try removing the files and see that you get the english fallback strings.
  • Make sure all untouched files still work (eg com_content) as before
  • Test overrides (eg using Language Override Manager)

Expected result

With this PR, everything should work as before, even when renaming the files.

Actual result

Without this PR you get the english fallback strings when you rename a file.

Points for Discussion

  • Do we want to rename the en-GB.ini file? For Crowdin it doesn't matter, it can rename that one since it can replace the full filename.
  • Do we want to deprecate the prefixed language files? If so, we need to communicate that early as it would impact language packs as well as every extension out there.
    Personally I would certainly keep it for 4.0 but it may be something to be removed for 5.0 but I don't think it's worth the trouble. The code to keep it both ways is quite simple.

Documentation Changes Required

Documentation regarding translations need to be adjusted.

@Bakual
Copy link
Contributor Author

Bakual commented Jan 11, 2018

For those wondering how Crowdin integrates GitHub, I have setup a test project for the Weblinks extensions: https://crowdin.com/project/joomla-weblinks-extension

The result is https://github.com/Bakual/weblinks/pull/2 which is all done by Crowdin itself automatically.

@mbabker
Copy link
Contributor

mbabker commented Jan 11, 2018

When I tried to use the GitHub integration of Crowdin, I faced the issue that Crowdin can't rename part of the translated filename.

This is basically why even when trying to use Crowdin's native CLI functionality I ended up publishing https://github.com/joomla-projects/crowdin-sync instead (making use of the same YAML config file). Through its tooling there just isn't a way to get the files downloaded with the correct name, but with the API you can manipulate the data to build the right file paths.

Removing the locale code from the filenames simplifies a lot of things in the long run IMO.

@brianteeman
Copy link
Contributor

i guess that the en-GB.ini could be renamed as joomla.ini

@Bakual
Copy link
Contributor Author

Bakual commented Jan 11, 2018

i guess that the en-GB.ini could be renamed as joomla.ini

From a code point of view, that actually would make sense as it makes the code simpler. We could get rid of the check $internal = $extension == 'joomla'.

@ot2sen
Copy link
Contributor

ot2sen commented Jan 11, 2018

Am in two minds about this one. Can see how it could ease things at a technical level. At the same time it looks like trouble leaving out the unique identifier for all the people dealing with language maintenance beyond one language.

When you work in a role as language coordinator, translation maintainer or the like, there would be a great mix of how your staff or volunteers would prefer to work with the language sets, the tools they use and how they deliver them back for use. Cutting out the file naming identifier could make such maintenance more troublesome. Having a visible file prefix identifier is rather helpful when dealing with multiple languages (2 -> +50)
Not sure if it is the dot separator causing the trouble for this specific crowdin case, if so perhaps an underscore change would solve that? (en-GB_com_extension.ini)

Another concern would be when installing multiple languages with an extension and making use of system wide language folders and also of in extension level language folders, to have different messages in the sys.inis for installation message and for the edit extension main message. Here you could have an install folder with multiple languages that are prefixed in same folder, and this would be a problem should they all lack their unique xx-XX identifier.

@mbabker
Copy link
Contributor

mbabker commented Jan 11, 2018

Not sure if it is the dot separator causing the trouble for this specific crowdin case, if so perhaps an underscore change would solve that?

It's not the dot separation. You upload a file as en-GB.com_extension.ini, regardless of the language you are downloading as you get a file named en-GB.com_extension.ini unless you write custom tooling like the one I'm using for various projects to automate fixing the locale code. This is a Crowdin problem in general (we have the same type of tooling workaround in the issue tracker which isn't using the Joomla Language API).

Another concern would be when installing multiple languages with an extension and making use of system wide language folders and also of in extension level language folders

Not a concern with the design of our language system. It enforces a <APPLICATION_OR_EXTENSION_ROOT>/language/<code>/<code>.<extension>.ini filesystem structure, making it impossible to put files of multiple languages into one directory.

@infograf768
Copy link
Member

i guess that the en-GB.ini could be renamed as joomla.ini

From a code point of view, that actually would make sense as it makes the code simpler. We could get rid of the check $internal = $extension == 'joomla'.

In that case, it should not be limited to en-GB obviously.

@Bakual
Copy link
Contributor Author

Bakual commented Jan 12, 2018

In that case, it should not be limited to en-GB obviously.

Not sure what you mean. Do you mean it has to be done for the other languages as well? If so, then yes.
It certainly needs a time where both variants are supported as well. Eg during 4.0 we allow both "joomla.ini" and "en-GB.ini" (or eg de-DE.ini), and with 5.0 it has to be joomla.ini.
That would give translators ample time to rename the file. Do you think that would work?

@ot2sen I understand that it might be harder to differentiate the files, but I think that could be solved by adjusting the workflow slightly. Eg put the files into subdirectories/zips with the proper language code.
On the other hand, you no longer have to rename the files when copying the english files to a new language. Or when copying the files to a dialect of the same language (eg de-DE to de-CH).

@infograf768
Copy link
Member

Some thoughts

  1. The patch works fine
  2. I agree with some aspects of @ot2sen remarks. I am concerned, as he is, about the impossibility to differentiate the files when a Team is exchanging files in various languages.
  3. There is also an aspect which I find troublesome is the fact that it is Joomla which has to adapt to Crowdin.
    I do understand that we evidently have to comply to W3C standards (including Accessibility) but in this case we make ourselves again dependent, code wise, from a closed source commercial system.
    Evidently I am known to be part of those that think Joomla should be as agnostic as possible to a lot of stuff, including bootstrap, but nevertheless...

Side Note:
concerning

On the other hand, you no longer have to rename the files when copying the english files to a new language. Or when copying the files to a dialect of the same language (eg de-DE to de-CH).

That is something com_localise does in a few clicks and then make the lang pack also in a few clicks. 😺

@Bakual
Copy link
Contributor Author

Bakual commented Jan 12, 2018

There is also an aspect which I find troublesome is the fact that it is Joomla which has to adapt to Crowdin.

Actually, I think other translation platforms may face the same issue if they have a GitHub integration.
Technically, there is no need for those prefixes as we have. I think this comes from history where maybe they all lived in the same folder in the beginning. If we would develop language support today, then we likely wouldn't add prefixes to the filenames.
But anyway, it's not strictly about Crowdin. It started there, yes, but my next thought was "Why do we even have this prefixes and could we do without to make things simpler and work also better with Crowdin". Sounded like a win-win to me so far.

That is something com_localise does in a few clicks and then make the lang pack also in a few clicks.

You could make the com_localise code simpler then as well. 😄

@infograf768
Copy link
Member

I think this comes from history where maybe they all lived in the same folder in the beginning.

Just for history, they already did reside in xx-XX folders in 1.5.

@Bakual
Copy link
Contributor Author

Bakual commented Jan 12, 2018

True, and in 1.0 it was a /language/english.php file. So it was like this since the INI files were introduced.

@brianteeman
Copy link
Contributor

All these "problems" are just "what ifs" and should never be an issue when using a robust workflow

@ot2sen
Copy link
Contributor

ot2sen commented Jan 12, 2018

@mbabker I probably wasn´t clear about the

Not a concern with the design of our language system. It enforces a <APPLICATION_OR_EXTENSION_ROOT>/language/<code>/<code>.<extension>.ini filesystem structure, making it impossible to put files of multiple languages into one directory.

where I meant within the installation package, not where it get installed.

Today you can have an installation XML like:

	<folder>language</folder>
</files>
<languages folder="lang">
    <language tag="en-GB">en-GB.plg_content_example.ini</language>
	<language tag="en-GB">en-GB.plg_content_example.sys.ini</language>
    <language tag="da-DK">da-DK.plg_content_example.ini</language>
	<language tag="da-DK">da-DK.plg_content_example.sys.ini</language>
	<language tag="it-IT">it-IT.plg_content_example.ini</language>
	<language tag="it-IT">it-IT.plg_content_example.sys.ini</language>
</languages>
<config><fields name="params"><fieldset name="basic" label="basic">


Actual content of /lang folder is listed side by side in the installation package:
/lang
en-GB.plg_content_example.ini
en-GB.plg_content_example.sys.ini
da-DK.plg_content_example.ini
da-DK.plg_content_example.sys.ini
it-IT.plg_content_example.ini
it-IT.plg_content_example.sys.ini

and because of the tag they get installed correctly in their respective prefix folders under the site wide language folder.

Without the prefix you would have no way to tell which is which of the 3 double sets of files.
Could this be a matter of documentation to ensure they are placed in subfolders just as done at the installation package for those under language in same XML.
Just to have awareness that it is possible to have them side by side in an installation package today before installation, and this wouldn´t work when file prefixes are gone. Might not be a bad thing :)

@Fedik
Copy link
Member

Fedik commented Jan 12, 2018

Today you can have an installation XML like

but you also can have just <folder>language</folder> and do not worry about any prefixes 😉

@Bakual
Copy link
Contributor Author

Bakual commented Jan 12, 2018

@ot2sen Personally, I wouldn't remove the prefixes in J4.0 anyway. So you could still do that in 4.0. If we remove it in 5.0, then that would be no longer possible.
However as Fedik said, it's easier anyway to just put the files into the respective subfolders and be done with a single folder tag.

@ot2sen
Copy link
Contributor

ot2sen commented Jan 12, 2018

@Fedik yeah, also did have the language in the example which would have subfolders with the prefixes within in the installation package. Those to go directly language folder inside the extension when installed, which is fine. The other /lang folder would have been more clear in purpose had I added just the sys.inis to demonstrate these are for having a different message for the user on installation finalisation. Anyway, it all comes down to habit of having done year of manual preparation. With automatisation it is not much of an issue ;)

@Bakual yep, workflow again. Automate with subfolders and it is not much of an issue. As I said in first post I do see how this can ease thing at the technical level. Dusting off the dinosaur and shaking off my manual habits. At the end of the day I like the suggestion :)

@mbabker
Copy link
Contributor

mbabker commented Jan 12, 2018

we make ourselves again dependent, code wise, from a closed source commercial system.

Honestly, having an opinion on this or making a decision shouldn't be 100% driven by Crowdin. With that said, just shooting from the hip here are how other systems handle filesystem structure.

Joomla: language/<locale>/<locale>.<extension>.ini
Symfony 4: translations/<extension>.<locale>.<file_format>
Laravel: resources/lang/<locale>/<extension>.php
Concrete 5: packages/<package>/languages/<locale>/LC_MESSAGES/messages.po

So Joomla's really the only system with a double locale code in its filesystem structure. Personally I'm not a fan of it but I see why it has benefits to some.

@zero-24 zero-24 added the RFC Request for Comment label Jan 19, 2018
@laoneo
Copy link
Member

laoneo commented Apr 4, 2018

Should we not target that pr against 4 and rename the language files correctly without the double locale?

@Bakual
Copy link
Contributor Author

Bakual commented Apr 4, 2018

Since it's completely B/C, it can (and imho should) be merged into 3.9.
We should then rename the language files for 4.0 of course. But the support should be in 3.9 so extensions could work in both with non-prefixed files.

@laoneo
Copy link
Member

laoneo commented Apr 4, 2018

True.

@infograf768
Copy link
Member

Is it really totally B/C? I have not tested with overrides. Has anybody done it? In overrides, the files are NOT separated by folders.

@mbabker
Copy link
Contributor

mbabker commented Apr 4, 2018

Considering the code loading and parsing overrides if found hasn't changed, then no, the overrides aren't broken (in part because it doesn't even use this same load method but bypasses it completely and goes straight to parse).

@Bakual
Copy link
Contributor Author

Bakual commented Apr 4, 2018

I actually tested overrides and they still work as expected. It's even mentioned as test case in the PR 😉

@infograf768
Copy link
Member

infograf768 commented Apr 5, 2018

Indeed, as the code concerning loading overrides ini has not changed, these are still working as are.

Suggestion: if we go this way then we may also create an array (on the model of localise.php) for
$filename = JPATH_BASE . "/language/overrides/$lang.override.ini";
to let load JPATH_BASE . "/language/overrides/$lang/override.ini

Some more remarks:
joomla.ini would work to replace en-GB.ini
something like (maybe also an array and KSort?)

		if ($internal)
		{
			$filenames[] = "$path/$lang.ini";
			$filenames[] = "$path/joomla.ini";  // NEW
		}
		else

---ADDED: Or check the existence of the file as done below for joomla.xml

Add this to the PR?

What would be the name in the future for en-GB.xml in the language packs? joomla.xml is already used as manifest.
My feeling is that we should first decide about all these aspects before merging this.

@laoneo
Copy link
Member

laoneo commented Apr 5, 2018

Should we target the pr against 3.9?

@Bakual Bakual force-pushed the SupportNonPrefixedLanguageFiles branch from 42f6d59 to 769a1de Compare January 15, 2019 19:06
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Jan 15, 2019
@Bakual Bakual changed the base branch from staging to 3.10-dev January 15, 2019 19:07
@Bakual Bakual added this to the Joomla 3.10.0 milestone Jan 15, 2019
Co-Authored-By: Bakual <werbemails@bakual.ch>
@joomla-cms-bot joomla-cms-bot added PR-3.10-dev and removed Language Change This is for Translators labels Jan 15, 2019
@Bakual
Copy link
Contributor Author

Bakual commented Jan 15, 2019

@wilsonge Rebased and changed base of PR. There were no conflicts.
I've merged some codestyle suggestions from @Quy. Only some superfluous spaces.

@zero-24 zero-24 removed their request for review January 15, 2019 19:21
@Bakual Bakual changed the title [RFC] Add support for non prefixed language files Add support for non prefixed language files Jan 17, 2019
@joomla-cms-bot joomla-cms-bot removed the RFC Request for Comment label Jan 17, 2019
@wilsonge wilsonge merged commit a0a42ba into joomla:3.10-dev Jan 21, 2019
@wilsonge
Copy link
Contributor

Thanks for this!

@zero-24
Copy link
Contributor

zero-24 commented Sep 29, 2020

This has been documented here: https://docs.joomla.org/J3.x:Joomla_3.10_Backports

akunzai added a commit to akunzai/joomla-external-login that referenced this pull request Jul 6, 2023
akunzai added a commit to akunzai/MageBridgeCore that referenced this pull request Jul 6, 2023
akunzai added a commit to akunzai/joomla-external-login that referenced this pull request Jul 16, 2023
akunzai added a commit to akunzai/MageBridgeCore that referenced this pull request Jul 16, 2023
akunzai added a commit to akunzai/joomla-external-login that referenced this pull request Jul 16, 2023
akunzai added a commit to akunzai/MageBridgeCore that referenced this pull request Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet