Support for non-string placeholders

[csicar]

Currently, tr has the type:

tr : Translations -> Delims -> String -> Replacements -> String

This allows the user to easily insert Strings into placeholder positions. I found, that having support for inserting e.g. HTML into placeholders often be useful. For example, take the translation string "Visit {{linkToPage}} for more information". linkToPage should be a placeholder for an HTML.a element, which cannot be represented with the current tr.

An alternative would be to split the translation string into multiple translation strings: visitPage1 and visitPage2. This detracts from the readability and is not friendly to translators, who need to infer from context how the sentence will be assembled. In addition, the programmer neglect other the grammar of other languages. If the programmer forgets to add a translation string for before and after the placeholder, a translator might not be able to form a proper sentence in the translated language.

Proposal:

type CustomReplacements a = List (String, a)

customTr : (String -> a) -> Translations -> Delims -> String -> CustomReplacements a -> List a

The first argument would tell customTr how to lift the translated part of the string to the a type. This would typically be Html.text or Element.text.

Usage:

-- If your translations are { "greet": "Hello {{name}}!" }
span [] <| customTr Html.text translations Curly "greet" [ ( "name", a [ href "..." ] [ Html.text "Peter" ] ) ]
-- equivalent to
span [ Html.text "Hello ", a [ href "..." ] [ Html.text "Peter" ], Html.text "!"]

[ChristophP]

Hi @csicar , hey cool thanks for the proposal. I really like the idea. I totally agree about splitting translation strings being very unfriendly to translators. Also your proposal seems both flexible enough to support all use cases while also making the overall package API not a lot more confusing or bloated. I only had a brief moment to look now but I'll try to take a closer look sometime throughout this week and think about a possible addition to this lib.

[ChristophP]

Hi @csicar,

thanks for going ahead and providing and initial implementation for this issue and sorry that I haven't gotten back. Things have been pretty busy at work.

The implementation looks good to me and having the tests is also good.

I have two more questions:

• You mentioned that parsing into a list should be added to the decoder? Is that necessary in your eyes? I thought the tokenized list was just an implementation detail of the customTr parsing.
• To mirror the API of the exisisting functions, what do you think about adding a customTrf function as well to be able to handle fallback languages?

[csicar]

thanks for going ahead and providing and initial implementation for this issue and sorry that I haven't gotten back. Things have been pretty busy at work.

No need to apologize :)

The implementation looks good to me and having the tests is also good.

Nice! Before merging this, I would also like to add more tests. Just to be sure, that the parsing can not be broken.

I have two more questions:

You mentioned that parsing into a list should be added to the decoder? Is that necessary in your eyes? I thought the tokenized list was just an implementation detail of the customTr parsing.

It is definitely not necessary to add the parsing step to the json decoder. Doing so would also require some breaking changes to the API. So I think it's fine to consider it an implementation detail.

To mirror the API of the exisisting functions, what do you think about adding a customTrf function as well to be able to handle fallback languages?

Yes definitely. I wanted to get your opinion on the initial implementation first, before implementing further functions. I guess only customTrf is missing right? I'll probably get to implementing the rest this Friday.

[ChristophP]

So I think it's fine to consider it an implementation detail.

Great so we can just leave that out of the exposed values for now.

Nice! Before merging this, I would also like to add more tests. Just to be sure, that the parsing can not be broken.

That's a good idea. I was wondering why there's a lot of parsing code needed at first. It's because it's not possible to use String.replace like in the string case right?

Yes definitely. I wanted to get your opinion on the initial implementation first, before implementing further functions. I guess only customTrf is missing right? I'll probably get to implementing the rest this Friday.

That sounds awesome. Yes in my opinion it's only customTrf that's missing.

[csicar]

It's because it's not possible to use String.replace like in the string case right?

Yes exactly. I use String.split instead. So Hello {{firstname}} {{lastname}} gets parsed into [Text "Hello ", Placeholder "firstname", Text " {{lastname}}"]. Then String.split is called on each Text element and concatenated afterwards. Thus the result is [Text "Hello ", Placeholder "firstname", Text " ", Placeholder "lastname"]

BTW: customTrf is implemented now

[ChristophP]

@csicar Great thank you. I merged the PR and started adding documentation. As I was doing that I noticed that I'd liek to change the argument order of customTr and customTrf to

1. Maintain consistency with other functions
2. To clarify the usage more. Can you please check this commit to see if this makes sense to you? https://github.com/ChristophP/elm-i18next/commit/e874db80a4b091989ad50df02ce9d573393466a2

[csicar]

Yes, I can see why that is more consistent.

I use elm-i18next through elm-i18next-gen so the argument order is not directly relevant to me. Still I think having lift before the translation key would be useful to users. That way you could setup your own per-configured translation function: E.g.myTr = customTr translations Curly Html.text, which would be used as myTr "greet" [.. ] (Of course you could reorder the arguments in myTr by accepting all arguments and then passing them in a different order, but that seems needlessly cumbersome) Basically, I would not expect the user to change the lift argument often - if ever - therefore I wanted to place it at the front.

[ChristophP]

@csicar Ah, that's a good point. I can see how that would be useful for partial application and that it may make more sense to have lift before passing translations and delimiter options.

Some of my thinking for moving lift to the back was that it was kind of hard to understand intuitively what lift does. the term lift will tell people with deeper FP knowledge that "it lifts something to a different type" but what the average user needs to understand that this argument will "wrap the parts which aren't placeholders", right?

[ChristophP]

I felt like moving it closer to the CustomReplacements makes it more clear that lift kind belongs to whatever isn't handled in the CustomReplacements. What's your take on that?

So to me it basically comes down to conveniance vs clarity. I see your point about the conveniant currying order. I think if we go that way we should take extra care to explain the lift argument well.

One more idea that I had is the following and it goes along well with what I said earlier about lift being related to CustomReplacements. What if lift was actually part of custom replacements?

[ChristophP]

Like

type alias CustomReplacements a = {
  placeholders: List (String, a),
  rest: String -> a
}

Would be cool because this would also reduce the argument count of customTr from 5 to 4 but I don't know if that's more conveniant than the other option.

[ChristophP]

Ah what the heck. I've decided to revert the changes of moving the argument and improve the docs.

[ChristophP]

Ok final change, I will go with this:

customTr : Translations -> Delims -> (String -> a) -> String -> CustomReplacements a -> List a
customTr translations delims lift translationKey replacements =

That way I can still preserve some consitency with other functions by keeping Translations as the first args followed by delims. But it would still play well with currying because I suspect lift won't change more than translations or delims will. So it could still be partially applied like this:

let translate = customTr translations Curly Html.text
in translate "some.key" ["link", a [href ""] [text ""]]

[ChristophP]

Released as 4.2.0 thanks for the suggestion and the help :-) I hope this is useful for you. https://github.com/ChristophP/elm-i18next/releases/tag/4.2.0

[csicar]

Absolutely. Thank you!