Skip to content

Commit 1755713

Browse files
stasmstasm
committed
Bug 1412808 - Remove the LITERAL helper.
Its most common use-case can be fullfilled with a simple FTL.TextElement() because CONCAT is also capable of concatenating TextElements and Placables. Its other use-case (defining a hard-coded value for a Message) is rare and the alternative is not too bad: FTL.Pattern([ FTL.TextElement("Hardcoded value") ]) Its third use-case was in REPLACE as the foreach function. That's already covered by the default value of foreach, however.
1 parent ec71937 commit 1755713

File tree

11 files changed

+61
-67
lines changed

11 files changed

+61
-67
lines changed

‎fluent/migrate/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
Source, COPY, REPLACE_IN_TEXT, REPLACE, PLURALS, CONCAT
66
)
77
from .helpers import ( # noqa: F401
8-
LITERAL, EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
8+
EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
99
)
1010
from .changesets import convert_blame_to_changesets # noqa: F401

‎fluent/migrate/context.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class MergeContext(object):
4141
- A list of `FTL.Message` objects some of whose nodes are special
4242
helper or transform nodes:
4343
44-
helpers: LITERAL, EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
44+
helpers: EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
4545
transforms: COPY, REPLACE_IN_TEXT, REPLACE, PLURALS, CONCAT
4646
"""
4747

‎fluent/migrate/helpers.py‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@
1313
import fluent.syntax.ast as FTL
1414

1515

16-
def LITERAL(value):
17-
"""Create a Pattern with a single TextElement."""
18-
elements = [FTL.TextElement(value)]
19-
return FTL.Pattern(elements)
20-
21-
2216
def EXTERNAL_ARGUMENT(name):
2317
"""Create an ExternalArgument expression."""
2418

‎fluent/migrate/transforms.py‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,44 +8,44 @@
88
All Transforms evaluate to Fluent Patterns. This makes them suitable for
99
defining migrations of values of message, attributes and variants. The special
1010
CONCAT Transform is capable of joining multiple Patterns returned by evaluating
11-
other Transforms into a single Pattern. It can also concatenate Fluent
12-
Expressions, like MessageReferences and ExternalArguments.
11+
other Transforms into a single Pattern. It can also concatenate Pattern
12+
elements: TextElements and Placeables.
1313
1414
The COPY, REPLACE and PLURALS Transforms inherit from Source which is a special
1515
AST Node defining the location (the file path and the id) of the legacy
1616
translation. During the migration, the current MergeContext scans the
1717
migration spec for Source nodes and extracts the information about all legacy
18-
translations being migrated. Thus,
18+
translations being migrated. For instance,
1919
2020
COPY('file.dtd', 'hello')
2121
2222
is equivalent to:
2323
24-
LITERAL(Source('file.dtd', 'hello'))
24+
FTL.Pattern([
25+
FTL.TextElement(Source('file.dtd', 'hello'))
26+
])
2527
26-
where LITERAL is a helper defined in the helpers.py module for creating Fluent
27-
Patterns from the text passed as the argument.
28-
29-
The LITERAL helper and the special REPLACE_IN_TEXT Transforms are useful for
30-
working with text rather than (path, key) source definitions. This is the case
31-
when the migrated translation requires some hardcoded text, e.g. <a> and </a>
32-
when multiple translations become a single one with a DOM overlay.
28+
Sometimes it's useful to work with text rather than (path, key) source
29+
definitions. This is the case when the migrated translation requires some
30+
hardcoded text, e.g. <a> and </a> when multiple translations become a single
31+
one with a DOM overlay. In such cases it's best to use the AST nodes:
3332
3433
FTL.Message(
3534
id=FTL.Identifier('update-failed'),
3635
value=CONCAT(
3736
COPY('aboutDialog.dtd', 'update.failed.start'),
38-
LITERAL('<a>'),
37+
FTL.TextElement('<a>'),
3938
COPY('aboutDialog.dtd', 'update.failed.linkText'),
40-
LITERAL('</a>'),
39+
FTL.TextElement('</a>'),
4140
COPY('aboutDialog.dtd', 'update.failed.end'),
4241
)
4342
)
4443
4544
The REPLACE_IN_TEXT Transform also takes text as input, making in possible to
4645
pass it as the foreach function of the PLURALS Transform. In this case, each
4746
slice of the plural string will be run through a REPLACE_IN_TEXT operation.
48-
Those slices are strings, so a REPLACE(path, key, …) isn't suitable for them.
47+
Those slices are strings, so a REPLACE(path, key, …) wouldn't be suitable for
48+
them.
4949
5050
FTL.Message(
5151
FTL.Identifier('delete-all'),
@@ -66,7 +66,12 @@
6666
from __future__ import unicode_literals
6767

6868
import fluent.syntax.ast as FTL
69-
from .helpers import LITERAL
69+
70+
71+
def pattern_from_text(value):
72+
return FTL.Pattern([
73+
FTL.TextElement(value)
74+
])
7075

7176

7277
def evaluate(ctx, node):
@@ -120,7 +125,7 @@ class COPY(Source):
120125

121126
def __call__(self, ctx):
122127
source = super(self.__class__, self).__call__(ctx)
123-
return LITERAL(source)
128+
return pattern_from_text(source)
124129

125130

126131
class REPLACE_IN_TEXT(Transform):
@@ -210,10 +215,12 @@ class PLURALS(Source):
210215
Build an `FTL.SelectExpression` with the supplied `selector` and variants
211216
extracted from the source. The source needs to be a semicolon-separated
212217
list of variants. Each variant will be run through the `foreach` function,
213-
which should return an `FTL.Node` or a `Transform`.
218+
which should return an `FTL.Node` or a `Transform`. By default, the
219+
`foreach` function transforms the source text into a Pattern with a single
220+
TextElement.
214221
"""
215222

216-
def __init__(self, path, key, selector, foreach=LITERAL):
223+
def __init__(self, path, key, selector, foreach=pattern_from_text):
217224
super(self.__class__, self).__init__(path, key)
218225
self.selector = selector
219226
self.foreach = foreach
@@ -264,7 +271,7 @@ def concat_elements(acc, cur):
264271
return acc
265272

266273
raise RuntimeError(
267-
'CONCAT accepts FTL Patterns and Expressions.'
274+
'CONCAT accepts FTL Patterns, TextElements and Placeables.'
268275
)
269276

270277
# Merge adjecent `FTL.TextElement` nodes.

‎tests/migrate/test_concat.py‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,8 @@
1010
DTDParser = PropertiesParser = None
1111

1212
from fluent.migrate.util import parse, ftl_message_to_json
13-
from fluent.migrate.helpers import (
14-
LITERAL, EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
15-
)
16-
from fluent.migrate.transforms import (
17-
evaluate, CONCAT, COPY, REPLACE
18-
)
13+
from fluent.migrate.helpers import EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
14+
from fluent.migrate.transforms import evaluate, CONCAT, COPY, REPLACE
1915

2016

2117
class MockContext(unittest.TestCase):
@@ -134,9 +130,9 @@ def test_concat_literal(self):
134130
FTL.Identifier('update-failed'),
135131
value=CONCAT(
136132
COPY(self.strings, 'update.failed.start'),
137-
LITERAL('<a>'),
133+
FTL.TextElement('<a>'),
138134
COPY(self.strings, 'update.failed.linkText'),
139-
LITERAL('</a>'),
135+
FTL.TextElement('</a>'),
140136
COPY(self.strings, 'update.failed.end'),
141137
)
142138
)
@@ -157,21 +153,20 @@ def setUp(self):
157153
<!ENTITY channel.description.end " channel. ">
158154
''')
159155

160-
@unittest.skip('Parser/Serializer trim whitespace')
161156
def test_concat_replace(self):
162157
msg = FTL.Message(
163158
FTL.Identifier('channel-desc'),
164159
value=CONCAT(
165160
COPY(self.strings, 'channel.description.start'),
166-
EXTERNAL_ARGUMENT('channelname'),
161+
FTL.Placeable(EXTERNAL_ARGUMENT('channelname')),
167162
COPY(self.strings, 'channel.description.end'),
168163
)
169164
)
170165

171166
self.assertEqual(
172167
evaluate(self, msg).to_json(),
173168
ftl_message_to_json('''
174-
channel-desc = You are on the { $channelname } channel.
169+
channel-desc = You are on the { $channelname } channel.\u0020
175170
''')
176171
)
177172

@@ -200,7 +195,7 @@ def test_concat_replace(self):
200195
)
201196
}
202197
),
203-
LITERAL('<a>'),
198+
FTL.TextElement('<a>'),
204199
REPLACE(
205200
self.strings,
206201
'community.mozillaLink',
@@ -210,11 +205,11 @@ def test_concat_replace(self):
210205
)
211206
}
212207
),
213-
LITERAL('</a>'),
208+
FTL.TextElement('</a>'),
214209
COPY(self.strings, 'community.middle'),
215-
LITERAL('<a>'),
210+
FTL.TextElement('<a>'),
216211
COPY(self.strings, 'community.creditsLink'),
217-
LITERAL('</a>'),
212+
FTL.TextElement('</a>'),
218213
COPY(self.strings, 'community.end')
219214
)
220215
)

‎tests/migrate/test_context.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
from fluent.migrate.util import ftl, ftl_resource_to_json, to_json
1111
from fluent.migrate.context import MergeContext
12-
from fluent.migrate.helpers import LITERAL
1312
from fluent.migrate.transforms import COPY
1413

1514

@@ -37,7 +36,9 @@ def test_hardcoded_node(self):
3736
self.ctx.add_transforms('aboutDownloads.ftl', [
3837
FTL.Message(
3938
id=FTL.Identifier('about'),
40-
value=LITERAL('Hardcoded Value')
39+
value=FTL.Pattern([
40+
FTL.TextElement('Hardcoded Value')
41+
])
4142
),
4243
])
4344

‎tests/migrate/test_context_real_examples.py‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88

99
from fluent.migrate.util import ftl_resource_to_json, to_json
1010
from fluent.migrate.context import MergeContext
11-
from fluent.migrate.helpers import (
12-
LITERAL, EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
13-
)
11+
from fluent.migrate.helpers import EXTERNAL_ARGUMENT, MESSAGE_REFERENCE
1412
from fluent.migrate.transforms import (
1513
CONCAT, COPY, PLURALS, REPLACE_IN_TEXT, REPLACE
1614
)
@@ -301,9 +299,9 @@ def setUp(self):
301299
id=FTL.Identifier('update-failed'),
302300
value=CONCAT(
303301
COPY('aboutDialog.dtd', 'update.failed.start'),
304-
LITERAL('<a>'),
302+
FTL.TextElement('<a>'),
305303
COPY('aboutDialog.dtd', 'update.failed.linkText'),
306-
LITERAL('</a>'),
304+
FTL.TextElement('</a>'),
307305
COPY('aboutDialog.dtd', 'update.failed.end'),
308306
)
309307
),
@@ -329,7 +327,7 @@ def setUp(self):
329327
)
330328
}
331329
),
332-
LITERAL('<a>'),
330+
FTL.TextElement('<a>'),
333331
REPLACE(
334332
'aboutDialog.dtd',
335333
'community.mozillaLink',
@@ -339,11 +337,11 @@ def setUp(self):
339337
)
340338
}
341339
),
342-
LITERAL('</a>'),
340+
FTL.TextElement('</a>'),
343341
COPY('aboutDialog.dtd', 'community.middle'),
344-
LITERAL('<a>'),
342+
FTL.TextElement('<a>'),
345343
COPY('aboutDialog.dtd', 'community.creditsLink'),
346-
LITERAL('</a>'),
344+
FTL.TextElement('</a>'),
347345
COPY('aboutDialog.dtd', 'community.end')
348346
)
349347
),

‎tests/migrate/test_literal.py‎ renamed to ‎tests/migrate/test_copy.py‎

File renamed without changes.

‎tests/migrate/test_merge.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
from fluent.migrate.util import parse, ftl, ftl_resource_to_json
1414
from fluent.migrate.merge import merge_resource
15-
from fluent.migrate.helpers import LITERAL
1615
from fluent.migrate.transforms import COPY
1716

1817

@@ -64,7 +63,9 @@ def setUp(self):
6463
),
6564
FTL.Message(
6665
FTL.Identifier('about'),
67-
value=LITERAL('Hardcoded Value')
66+
value=FTL.Pattern([
67+
FTL.TextElement('Hardcoded Value')
68+
])
6869
),
6970
FTL.Message(
7071
FTL.Identifier('open-menuitem'),

‎tests/migrate/test_plural.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
PropertiesParser = None
1111

1212
from fluent.migrate.util import parse, ftl_message_to_json
13-
from fluent.migrate.helpers import LITERAL, EXTERNAL_ARGUMENT
13+
from fluent.migrate.helpers import EXTERNAL_ARGUMENT
1414
from fluent.migrate.transforms import evaluate, PLURALS, REPLACE_IN_TEXT
1515

1616

@@ -88,8 +88,7 @@ def setUp(self):
8888
value=PLURALS(
8989
self.strings,
9090
'deleteAll',
91-
EXTERNAL_ARGUMENT('num'),
92-
LITERAL
91+
EXTERNAL_ARGUMENT('num')
9392
)
9493
)
9594

0 commit comments

Comments
 (0)