basho / cuttlefish

never lose your childlike sense of wonder baby cuttlefish, promise me?
Apache License 2.0
205 stars 124 forks source link

To Kill A Translationbird #82

Closed joedevivo closed 10 years ago

joedevivo commented 10 years ago

If for some reason, you are overriding an existing schema with something particular for your project, you may have run into an edge case. If you need to override a translation, you're fine, but if a translation exists and you just need to remove it? You're out of luck... until now!

Now, you can do this:

{translation, 
 "some.key",
 fun(Conf) ->
   %% Do something to something and return translated value
}.

%% Oh no, that won't do at all
{translation, "some.key"}.

The second translation removes the first from the list and replaces it with an undefined function. It's important to note that if a third translation was added afterwards, it would override that deletion.

joedevivo commented 10 years ago

A fix for #79. This is a simple implementation that will ignore translations with undefined functions, and the ability to define those as {translation, Mapping}

I want to give a closer look at the stuff in PR #80 w.r.t to a better definition of the translation fun's API. Will dig in more later this week on that front.

cc: @Licenser

seancribbs commented 10 years ago

I see no problem with this, however I'm also not sure if it resolves #79. It allows the key to be ignored at schema-writing time, but not at config-parsing time.

joedevivo commented 10 years ago

@seancribbs this (https://github.com/basho/cuttlefish/commit/cafd9df2c7471dda24edb692154eb9439bdb4315#diff-e55ace98b9ea474b2b2bb3a984f6375cR69) is what makes it ignored at config-parsing time; it literally drops the existing one as it's read in.

seancribbs commented 10 years ago

Right, but i think what @Licenser meant was that a translation function could return a value that would prevent generation of the config entry. Overriding an existing translation in the schema doesn't accomplish that in the same way, you see?

Licenser commented 10 years ago

I actually tossed in a PR for that too. The issue is that sometimes a translation should not generate a value in the app.config since the mapping it reads from might not even be present (for example a optional mapping that needs to be translated if it exists).

joedevivo commented 10 years ago

Doesn't solve #79, but does solve some other edge case I thought #79 was. Merging 'cause @seancribbs said it was ok on mumble