Improve migration writing UX
Categories
(Localization Infrastructure and Tools :: Fluent Migration, enhancement)
Tracking
(Not tracked)
People
(Reporter: zbraniecki, Unassigned)
References
Details
Bug 1568133 gives us an example of a very hard to write migration code.
For example, this chunk:
FTL.Message(
id=FTL.Identifier('menu-quit'),
attributes=[
FTL.Attribute(
FTL.Identifier('label'),
FTL.Pattern(
elements=[
FTL.Placeable(
expression=FTL.SelectExpression(
selector=FTL.FunctionReference(
id=FTL.Identifier('PLATFORM'),
arguments=FTL.CallArguments([], [])
),
variants=[
FTL.Variant(
key=FTL.Identifier('windows'),
default=False,
value=REPLACE(
'browser/chrome/browser/browser.dtd',
'quitApplicationCmdWin2.label',
{
"&brandShorterName;": TERM_REFERENCE("brand-shorter-name"),
}
)
),
FTL.Variant(
key=FTL.Identifier('macos'),
default=False,
value=REPLACE(
'browser/chrome/browser/browser.dtd',
'quitApplicationCmdMac2.label',
{
"&brandShorterName;": TERM_REFERENCE("brand-shorter-name"),
}
)
),
FTL.Variant(
key=FTL.Identifier('other'),
default=True,
value=REPLACE(
'browser/chrome/browser/browser.dtd',
'quitApplicationCmd.label',
{
"&brandShorterName;": TERM_REFERENCE("brand-shorter-name"),
}
)
)
]
)
)
]
)
),
FTL.Attribute(
FTL.Identifier('accesskey'),
FTL.Pattern(
elements=[
FTL.Placeable(
expression=FTL.SelectExpression(
selector=FTL.FunctionReference(
id=FTL.Identifier('PLATFORM'),
arguments=FTL.CallArguments([], [])
),
variants=[
FTL.Variant(
key=FTL.Identifier('windows'),
default=False,
value=COPY(
'browser/chrome/browser/browser.dtd',
'quitApplicationCmdWin2.accesskey'
)
),
FTL.Variant(
key=FTL.Identifier('macos'),
default=False,
value=FTL.Pattern(elements=[FTL.Placeable(
expression=FTL.StringLiteral(
""
)
)])
),
FTL.Variant(
key=FTL.Identifier('other'),
default=True,
value=COPY(
'browser/chrome/browser/browser.dtd',
'quitApplicationCmd.accesskey'
)
)
]
)
)
]
)
),
]
),
would be way less prone to error and easier to write and review if it was encoded as:
menu-quit =
.label =
{ PLATFORM() ->
[windows] { REPLACE_BRANDS(base_path, "quitApplicationCmdWin2.label") }
[macos] { REPLACE_BRANDS(base_path, "quitApplicationCmdMac2.label") }
*[other] { REPLACE_BRANDS(base_path, "quitApplicationCmd.label") }
}
.accesskey =
{ PLATFORM() ->
[windows] { COPY(base_path, "quitApplicationCmdWin2.accesskey") }
[macos] { "" }
*[other] { COPY(base_path, "quitApplicationCmd.accesskey") }
}
Since brand replacement is the most common operation, we could provide a helper method, and the rest is just building up on top of the current templating.
Reporter | ||
Comment 1•5 years ago
|
||
Here's a mock of how the full migration script might look like:
# Any copyright is dedicated to the Public Domain.
# http://creativecommons.org/publicdomain/zero/1.0/
from __future__ import absolute_import
import fluent.syntax.ast as FTL
from fluent.migrate.helpers import transforms_from
from fluent.migrate import COPY, CONCAT, REPLACE
from fluent.migrate.helpers import MESSAGE_REFERENCE, TERM_REFERENCE, VARIABLE_REFERENCE
def migrate(ctx):
"""Bug 1568133 - Migrate remaining menubar from dtd to ftl, part {index}"""
ctx.add_transforms(
'browser/browser/menubar.ftl',
'browser/browser/menubar.ftl',
transforms_from(
"""
menu-application-services =
.label = { COPY(base_path, "servicesMenuMac.label") }
menu-application-hide-this =
.label = { REPLACE_BRANDS(base_path, "hideOtherAppsCmdMac.label") }
menu-application-hide-other =
.label = { COPY(base_path, "hideOtherAppsCmdMac.label") }
menu-application-show-all =
.label = { COPY(base_path, "showAllAppsCmdMac.label") }
menu-application-touch-bar =
.label = { COPY(base_path, "touchBarCmdMac.label") }
menu-quit =
.label =
{ PLATFORM() ->
[windows] { REPLACE_BRANDS(browser_path, "quitApplicationCmdWin2.label") }
[macos] { REPLACE_BRANDS(browser_path, "quitApplicationCmdMac2.label") }
*[other] { REPLACE_BRANDS(browser_path, "quitApplicationCmd.label") }
}
.accesskey =
{ PLATFORM() ->
[windows] { COPY(browser_path, "quitApplicationCmdWin2.accesskey") }
[macos] { "" }
*[other] { COPY(browser_path, "quitApplicationCmd.accesskey") }
}
menu-quit-button =
.label = { menu-quit.label }
.tooltip =
{ PLATFORM() ->
[windows] { REPLACE_BRANDS(browser_path, "quitApplicationCmdWin2.tooltip") }
*[other] { "" }
}
menu-about =
.label = { REPLACE_BRANDS(base_path, "aboutProduct2.label")
.accesskey = { COPY(base_path, "aboutProduct2.accesskey") }
""", base_path="browser/chrome/browser/baseMenuOverlay.dtd", browser_path="browser/chrome/browser/browser.dtd"))
Comment 2•5 years ago
|
||
Writing the linked migration was very painful and would have been even worse if Zibi wasn't sitting next to me. Adding some helpers for these common cases would go a long way.
Comment 3•5 years ago
|
||
Anything we could do to make this easier will pay off. Especially since as things currently stand these migrations will still be required even after initial migration to fluent (sinc revving a string in ftl requires writing a migration as well and devs are accustomed to not having this additional step with dtd/properties).
Comment 4•5 years ago
|
||
since revving a string in ftl requires writing a migration as well and devs are accustomed to not having this additional step with dtd/properties
This should only apply to refactors where value or attribute logic change (.label
-> .value
etc). That might be a thing right now, but surely won't be a lasting problem.
The regular changing of UI that requires new translations doesn't involve any new processes.
Comment 5•5 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #4)
since revving a string in ftl requires writing a migration as well and devs are accustomed to not having this additional step with dtd/properties
This should only apply to refactors where value or attribute logic change (
.label
->.value
etc). That might be a thing right now, but surely won't be a lasting problem.The regular changing of UI that requires new translations doesn't involve any new processes.
Thanks for the clarification, that's good to know. When changing a string without attribute logic changes, should we continue to rev the ID or change only the string value?
Comment 6•5 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #5)
Thanks for the clarification, that's good to know. When changing a string without attribute logic changes, should we continue to rev the ID or change only the string value?
Change the ID. See also
https://firefox-source-docs.mozilla.org/intl/l10n/l10n/fluent_review.html
Comment 7•5 years ago
|
||
As we're using fluent.migrate
in bedrock
now, too, it's more clear that we shouldn't build gecko-specific transforms into fluent.migrate
itself. Argueably, transforms.PLURALS
is also a good example of transforms that should probably live in mozilla-central
natively.
I just filed and started hacking around bug 1596726. Is that the right start to address the concerns here?
In particular, I'm envisioning that the gecko-specific subclass could watch out for
TermReference(id=Identifier(name='-brand*'), atttribute=None, arguments=None)
and generate a REPLACE
item instead of the TermReference
one. If it hooks itself up in Pattern
, I guess.
Comment 8•2 years ago
|
||
As far as I can tell, this was solved by adding the transforms_from()
helper.
Description
•