ChristophP / elm-i18next

https://package.elm-lang.org/packages/ChristophP/elm-i18next/latest
BSD 3-Clause "New" or "Revised" License
67 stars 13 forks source link

A way to compare translations #12

Closed allanderek closed 5 years ago

allanderek commented 5 years ago

When working on the translations for different languages it's helpful to be able to calculate if a given translation is "complete". To help with this I propose the addition of two functions to the API:

  1. keys: which returns all the translation keys defined by a given translation:

    keys : Translations -> List String
    keys translations =
     case translations of
         Translations d ->
              Dict.keys d
  2. diff : returns the keys that are defined in the first translation but not the second

    diff : Translations -> Translations -> List String
    diff left right =
    case (left, right) of
        (Translations leftD, Translation rightD) ->
             Dict.diff leftD rightD |> Dict.keys

    Though I guess the second is easy enough to define yourself given the first.

ChristophP commented 5 years ago

@allanderek I would like to keep the API of this package as general and as small as possible. So how about this alternative? We expose a function called toDict and then let the caller implement whatever diffing logic they need based on that.

toDict : Translations -> Dict
toDict (Translations dict) = dict
allanderek commented 5 years ago

That would of course also work for my use case. This is I guess more general, and would make further additions less likely to be necessary.

One other thing I had planned was checking if the translations define the same replacements, that is that they have the same "holes". Eg. It would be a mistake if the English greeting translated to "hello {{firstname}}" and the German one to "hallo {{erstename}}", so it would be useful to have a function to do this. Such things anyone can do themselves, but the point of a library is they don't have to. On the other hand it's often good to keep an API small. So it's your call.

On Thu, 6 Dec 2018, 19:18 ChristophP <notifications@github.com wrote:

@allanderek https://github.com/allanderek I would like to keep the API of this package as general and as small as possible. So how about this alternative? We expose a function called toDict and then let the caller implement whatever diffing logic they need based on that.

toDict : Translations -> DicttoDict (Translations dict) = dict

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChristophP/elm-i18next/issues/12#issuecomment-444995116, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwSiQROSPGi_uekGNuonWe9yrFOUrCKks5u2W1ogaJpZM4ZC7uK .

ChristophP commented 5 years ago

@allanderek I got another question about your use case. You said you wanted to check if a translation is "complete". Isn't that more of a task for some pre compile step? If you are running it at runtime, what is your next action when you discovered that it is or isn't complete?

The way I usually handle things is making sure that the translation is in fact complete before I deploy and "trust" that it is in my runtime code.

allanderek commented 5 years ago

That's a good question. I'm actually writing an application for the translators to use to provide their translations.

ChristophP commented 5 years ago

Ok, just wanted to know if moving that logic to compile time had any implications in the exposed API. But I think it would be a good idea to expose toDict either way.

ChristophP commented 5 years ago

Oh I see, so you are trying to tell your users (the translators) that there is a key missing that they need to add and translate. Am I correct?

toDict could be useful for that and a number of generic tasks. I merged your PR. Thanks for your contribution. I will add some tests and update some stuff and publish it as soon as I have it done.

allanderek commented 5 years ago

Yes that's correct. The toDict function absolutely will allow me to do this.

ChristophP commented 5 years ago

@allanderek I decided to go with your initial approach again after thinking about it for a while. I published everything here https://package.elm-lang.org/packages/ChristophP/elm-i18next/latest/ . Let me know what you think.

allanderek commented 5 years ago

Yes this is great. This lets me get on with the application for the translator and allows me to upgrade to elm/http@2.0.0. Thanks.