Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Added Language Switching #1285

Closed
wants to merge 10 commits into from
Closed

Added Language Switching #1285

wants to merge 10 commits into from

Conversation

jdiehl
Copy link
Contributor

@jdiehl jdiehl commented Jul 20, 2012

I noticed several people offering to do localization work. To help these people, I added a basic localization infrastructure to Brackets and an option to switch languages from the Debug menu.

Localizations are added to the strings folder using their ISO language and country names (en-US for American English). These localizations show up when initiating the switch language command. The language preference is stored in localStorage.

If things break (e.g., the language file is deleted after enabling it in Brackets), the user must manually reset the localStore (window.localStore.removeItem("language") ). This is no state that can occur under normal conditions but might happen as people play around with this.

All code is JSLinted.

@jdiehl
Copy link
Contributor Author

jdiehl commented Jul 20, 2012

Some quick comments:

strings.js dynamically loads the language strings from strings/{ISOCODE}.js and copies all exported symbols into itself. This way, there is no need to change the access to the strings from anywhere else.

The strings are loaded when the module is loaded using require([...], function(strings) {}). This way, the DOM pageLoaded event is not triggered before the current language strings are loaded.

Currently, Brackets must be restarted to load a different language. This is OK for now and can probably stay this way for the future (iOS / OS X apps behave the same way).

Language switching is located in the Debug menu because ideally, the correct language should be defined by the Shell-App according to the OS setting. Consequently, active language switching is only useful for debugging and authoring localizations and should be hidden from the end users.

@ghost ghost assigned RaymondLim Jul 24, 2012
@RaymondLim
Copy link
Contributor

@jdiehl

Thanks for taking the initiative in localization effort. We will review your changes after we have discussion in our next architectural meeting as we have some concerns of your current design.

@joelrbrandt joelrbrandt mentioned this pull request Jul 26, 2012
@peterflynn
Copy link
Member

@jdiehl: We discussed this a bit during today's architecture meeting. One concern is whether the asynchronous require() call means that other modules that call require("strings.js") might temporarily get an object that doesn't have the strings loaded yet. The Require & AMD docs seem pretty vague on this point, and the Require source code looks like it doesn't do anything to prevent this.

There are also some additional pieces of localization functionality that we might like to see before committing this to core. I think it'd be good to talk through the details on the discussion forum before going too far with any one implementation route -- so I'll start a thread on there momentarily.

But btw, thanks for doing all the work to translate the strings to German! That's awesome. I think between you and @pthiess, Brackets will wind up having great German locale support :-)

@pthiess
Copy link
Contributor

pthiess commented Jul 30, 2012

Thanks for your reply and committing myself. No kidding localization is an
important piece and we juggle a number of requirements in our minds.
Thanks for starting a discussion to share with others and see what demands
are out there. P -:)

On 7/30/12 4:43 PM, "Peter Flynn"
<reply+i-5732048-6a02650750ea883411dd74d8aa57f0c616dafc4d-1584287@reply.git
hub.com> wrote:

@jdiehl: We discussed this a bit during today's architecture meeting.
One concern is whether the asynchronous require() call means that other
modules that call require("strings.js") might temporarily get an object
that doesn't have the strings loaded yet. The Require & AMD docs seem
pretty vague on this point, and the Require source code looks like it
doesn't do anything to prevent this.

There are also some additional pieces of localization functionality that
we might like to see before committing this to core. I think it'd be
good to talk through the details on the discussion forum before going too
far with any one implementation route -- so I'll start a thread on there
momentarily.

But btw, thanks for doing all the work to translate the strings to
German! That's awesome. I think between you and @pthiess, Brackets will
wind up having great German locale support :-)


Reply to this email directly or view it on GitHub:
#1285 (comment)

@peterflynn
Copy link
Member

I've started a thread here: https://groups.google.com/forum/?fromgroups#!topic/brackets-dev/DnL_L4pejq4

Re the async issues -- @RaymondLim suggested a quick workaround: we statically load strings.js from a fixed location as before, and the locale-picker dialog overwrites the contents of that file before restarting Brackets. That'd be a simple way to get up and running, but we'd probably hit limitations with some of our other needs (like fallback or dynamically matching the OS's locale -- see forum post for more). In the long run, I'm hoping we can use something more like the Require "i18n" plugin.

@peterflynn
Copy link
Member

Oh, also... @jdiehl: Have you noticed any issues with the UI layout yet as you're running Brackets in German? Words are longer on average in German than in English, so it's a great language to spot problems where the UI makes too many assumptions about the width/height of text.

Conflicts:
	src/debug/DebugCommandHandlers.js
	src/strings.js
@jdiehl
Copy link
Contributor Author

jdiehl commented Jul 31, 2012

So far, I have not found any layout issues. We will need to be careful with dialog titles, as they will look crappy if they break lines. Further, most buttons are not localized, which also typically need careful consideration.

On 31.07.2012, at 01:59, Peter Flynn reply@reply.github.com wrote:

Oh, also... @jdiehl: Have you noticed any issues with the UI layout yet as you're running Brackets in German? Words are longer on average in German than in English, so it's a great language to spot problems where the UI makes too many assumptions about the width/height of text.


Reply to this email directly or view it on GitHub:
#1285 (comment)

@jdiehl
Copy link
Contributor Author

jdiehl commented Jul 31, 2012

@peterflynn: The correct strings file is now loaded using the i18n require plugin (which I added to thirdparty/). Since there is no more asynchronous stuff happening, this should now be safe.

I created a new (non-standard) exports variable in the original strings.js file (in English) to make it look as close as possible to any other strings.js files, such that contributors can use this as a basis for their localization.

exports.OPEN_DIALOG_ERROR = "An error occurred when showing the open file dialog. (error {0})";
exports.REQUEST_NATIVE_FILE_SYSTEM_ERROR = "An error occurred when trying to load the directory <span class='dialog-filename'>{0}</span>. (error {1})";
exports.READ_DIRECTORY_ENTRIES_ERROR = "An error occurred when reading the contents of the directory <span class='dialog-filename'>{0}</span>. (error {1})";
module.exports = require("thirdparty/i18n!nls/strings");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In brackets.js require.config() we should update to:

require.config({
  paths: {
    "text" : "thirdparty/text",
    "i18n" : "thirdparty/i18n"
  }
});

...so that this require call can be require("i18n!nls/strings")

@tvoliter tvoliter closed this Aug 15, 2012
@tvoliter
Copy link
Contributor

This code has now been merged into master via another pull request that combined code from this pull request with code that templatized and localized index.html with mustache.js. Closing this pull request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants