1493184 - Generalize trimming across all transforms

Status: RESOLVED FIXED

(Localization Infrastructure and Tools :: Fluent Migration, enhancement)

Description

[Staś Małolepszy :stas]

Bug 1491833 added TRIM_COPY which may be used to trim leading and trailing whitespace in translations migrated from DTDs. I'd like to propose a more general solution which will also work for other transforms, most notably for REPLACE:

    COPY(path, key, trim=True)
    REPLACE(path, key, substs, trim=True)
    etc.

Comment 1

[Staś Małolepszy :stas]

Attachment: Bug 1493184 - Remove TRIM_COPY in favor of generalized Source(trim=True)

Comment 2

[Axel Hecht [:Pike]]

Comment on attachment 9010969 [details]
Bug 1493184 - Remove TRIM_COPY in favor of generalized Source(trim=True)

Axel Hecht [:Pike] has approved the revision.

Comment 3

[Staś Małolepszy :stas]

Thanks, Axel. I have one more question wrt. to the proposed API. I'm not particularly fond of using two special strings, "True" and "False", to represent booleans:

    COPY("path", "key", trim: "True")

I chose this design because this special casing is limited in scope: it's only valid in transforms_from, which admittedly is already a bit of a snowflake. The capitalization mimics the Python syntax for this reason, too. It's only supposed to work with migration recipes which are a mix of Python and FTL.

I have second thoughts about this. Perhaps adding a "mode" with a string-typed value would be a more bullet-proof design?

    COPY("path", "key", mode: "trim")

Do you have a preference?

Comment 4

[Axel Hecht [:Pike]]

That was a bit of a concern to me, too. I wondered if we should do `lower-case equals true, yes or 1`, or something like that.

But I wasn't overly concerned about that, and figured I'd go without a comment on that.

`mode` oth also has its downsides. Like, what if you want two independent modes? Then we get into the whole jazz of wordlist matching ;-)

I'd stick with a per-purpose keyword arg. Random distracting idea, using "on" and "off" generates sentences like `trim is on` or `trim is off`, and might get you a bit out of the mess of programming-language-specific boolean conversions.

Comment 5

[Staś Małolepszy :stas]

YAML uses the following regex (from http://yaml.org/type/bool.html):

     y|Y|yes|Yes|YES|n|N|no|No|NO
    |true|True|TRUE|false|False|FALSE
    |on|On|ON|off|Off|OFF

I'm happy to go with just "on" and "off", although it doesn't work as well for options whose names start with verbs: should_foo or has_bar probably work better with "true" and "false".

Comment 6

[Staś Małolepszy :stas]

Given that all recipes are reviewed by flod, I went with his preference: "True" and "False". Landed in https://hg.mozilla.org/l10n/fluent-migration/rev/8131ed2aff8d78fec7a34d542ef2924af1a9aa6e.