Introduce run-time language switching#11
Conversation
by introducing types describing their intent.
This incorporates all present translations into one build. Instead of a symlink to the compile-time language, a switch is generated for every translation module. This switch contains all the functions of the original(s), but they take a Language as an extra (preceeding) argument, delegating the translation itself to the corresponding module for that language. Migration of apps is needed, instructions in README.md. NOTE: This breaks the currently implemented behaviour of "one language per build".
|
Hi Niklas, thank you for your contribution! This looks very promising. We will try our best to get your code reviewed ASAP. |
|
The test appear to be failing due to an additional newline introduced here. |
felixLam
left a comment
felixLam
left a comment
There was a problem hiding this comment.
Just some very general feedback on coding-style. More substantive feedback will follow.
| elm-i18n-generator --language De,Pl,En,Fr --genswitch | ||
| ``` | ||
|
|
||
| #### Migrate legacy symlinked translations to switchables |
There was a problem hiding this comment.
We should maybe not just yet call it legacy. I believe that both options could reside side-by-side with advantages for each approach.
There was a problem hiding this comment.
Ideally we need to implement this in a way that does not break existing implementation unless they use new cli flags.
|
|
||
| import CSV.Template | ||
| import Localized | ||
| import Localized exposing (..) |
There was a problem hiding this comment.
Please revert changes to this file. There do not appear to be substantive changes to this file. We avoid using exposing (..). We usually choose module names and exposed names to go well together: Localized.Element is much clearer than Element IMHO.
There was a problem hiding this comment.
This might become quite "loud" because of the types I introduced. While it is easier to find the location of the definition, it forces a lot of repetition. I'll change it if you insist, but I prefer the slickness (TM).
There was a problem hiding this comment.
We side with https://github.com/stil4m/elm-analyse on this. Keeping this style-guide also reduce the size of the diff in this PR.
| import Csv | ||
| import Dict | ||
| import Localized | ||
| import Localized exposing (..) |
There was a problem hiding this comment.
I really appreciate the changes from String to named type alias! Please keep the named import though: again Localized.Module is easier to read and understand than just Module
|
|
||
|
|
||
| generateForModule : List (List String) -> List Localized.Element | ||
| -- Generate the source code for each module based on the lines |
There was a problem hiding this comment.
Are you using the latest version of elm-format?
|
I refactored quite a lot, hope it does not look too ruby-ish. The newline has been ele.. eli.. removed. |
As some claim these are easier to read. YMMV :)
|
Any news on this? |
|
I'm not co-maintaining this library anymore so can't give any updates. |
5 participants
This incorporates all present translations into one build as described in #2 . Instead of a symbolic link to the compile-time language, a switch is generated for every translation module. This switch contains all the functions of the original(s), but they take a Language as an extra (preceeding) argument, delegating the translation itself to the corresponding module for that language.
Migration of apps is needed, instructions in README.md.
NOTE: This breaks the currently implemented behaviour of "one language per build".
NOTE2: This might still contain some old behavior. I will clean that up if you want that PR :-)