1399057 - [compare-locales] Optionally yield Sections and Comments from FluentParser.walk

Status: RESOLVED FIXED

(Localization Infrastructure and Tools :: compare-locales, defect)

Description

[Staś Małolepszy :stas]

When onlyEntities is False, FluentParser should yield non-Message entries too, such as Comments and Sections.

Comment 1

[Staś Małolepszy :stas]

Attachment: Bug 1399057 - Yield Sections and Comments from FluentParser.walk.

Review commit: https://reviewboard.mozilla.org/r/187188/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/187188/

Comment 2

[Staś Małolepszy :stas]

This bug requires a new version of python-fluent due to bug 1406342.

Comment 3

[Axel Hecht [:Pike]]

Comment on attachment 8915929 [details]
Bug 1399057 - Yield Sections and Comments from FluentParser.walk.

https://reviewboard.mozilla.org/r/187188/#review192246

This is basically good, but we'll need a way to require the fluent update. I think cutting another release of python-fluent is the best way for that. With that and the change to setup.py to require that update, r=me.

Comment 4

[Staś Małolepszy :stas]

Thanks. I plan to release python-fluent 0.4.3 when bug 1397234 is fixed. it's already fixed in fluent.js so it's just a matter of porting it.

Comment 5

[Staś Małolepszy :stas]

https://hg.mozilla.org/l10n/compare-locales/rev/cbe6d883a8e90f73a62d9e5c6e980ea6cbc18ef0
Bug 1399057 - Yield Sections and Comments from FluentParser.walk. r?Pike

Comment 6

[Staś Małolepszy :stas]

I accidentally landed this (comment 5) and Axel backed it out in https://hg.mozilla.org/l10n/compare-locales/rev/c6e56d00f7d9.

Comment 7

[Staś Małolepszy :stas]

Comment on attachment 8915929 [details]
Bug 1399057 - Yield Sections and Comments from FluentParser.walk.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187188/diff/1-2/

Comment 8

[Staś Małolepszy :stas]

Comment on attachment 8915929 [details]
Bug 1399057 - Yield Sections and Comments from FluentParser.walk.

https://reviewboard.mozilla.org/r/187188/#review192666

::: compare_locales/tests/test_ftl.py:106
(Diff revisions 1 - 2)
>      C
>  ''')
>  
>          [abc] = list(self.parser)
>          self.assertEqual(abc.key, 'abc')
> -        self.assertEqual(abc.val, '\n    A\n    B\n    C')
> +        self.assertEqual(abc.val, '    A\n    B\n    C')

This is a fallout from bug 1397234 and I filed bug 1406880 to decide if it should be changed. For now I don't think it affects compare-locales much; it shouldn't block us from moving forward.

::: setup.py:48
(Diff revision 2)
>        packages=['compare_locales', 'compare_locales.tests'],
>        package_data={
>            'compare_locales.tests': ['data/*.properties', 'data/*.dtd']
>        },
>        install_requires=[
> -          'fluent>=0.4.2',
> +          'fluent==0.4.3',

The development of python-fluent is tightly connected to the needs of compare-locales these days  and there might be small changes to the AST, the reported spans etc. between minor versions.  Let's pin point the exact version of the dependency for now; we can relax this later on once everything stabilizes.

Comment 9

[Staś Małolepszy :stas]

https://hg.mozilla.org/l10n/compare-locales/rev/72372d88d8fc3a83caf8dc24d7b50828b2f27d5b
Bug 1399057 - Yield Sections and Comments from FluentParser.walk. r=Pike