Skip to content

Implement Syntax 0.8 #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 13, 2018
Merged

Implement Syntax 0.8 #78

merged 5 commits into from
Dec 13, 2018

Conversation

stasm
Copy link
Contributor

@stasm stasm commented Dec 6, 2018

@stasm
Copy link
Contributor Author

stasm commented Dec 7, 2018

@Pike, the changes in fluent/syntax are taken one-to-one from the JS implementation. There's one case where I needed to handle Python2 vs 3 explicitly: when unescaping escape sequences in StringLiterals, fluent.js uses String.fromCodePoint. In Python 2, we need unichr; in Python 3 that's simply chr.

I also realized that neither structure nor behavior tests have a test which would discover the need to use unichr vs. chr. So I added reference fixtures in a separate commit :) Two things to note:

  • Just as in fluent.js, we need to skip two reference fixtures (leading_dots.ftl and variant_lists.ftl) due to how the tooling parsers reject the whole message if an attribute is broken.
  • I added a processing function argument to BaseNode.to_json() so that I can remove the span property from the JSON object. I also use it to reset Junk annotations to [].

@stasm stasm requested a review from Pike December 7, 2018 11:56
Copy link
Contributor

@Pike Pike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, tested this locally a bit, gave it a quick run through c-l tests without surprises, too.

I did a side-by-side review of the parsers, only found a nit in the js side (issue filed).

I also compared the test fixtures, and they're nicely equal between fluent-syntax and python-fluent. There's one exception, and that's fixtures_structure/multiline_pattern, which I analyzed, and the error messages are different for bad message starts. fluent-syntax finds the missing =, python-fluent complains about value--attribute-less messages. That could also go away with something along the lines of #77.

@stasm
Copy link
Contributor Author

stasm commented Dec 7, 2018

Thanks! I'll merge this on Monday.

The fixtures differences are documented in https://github.com/projectfluent/python-fluent/blob/master/tests/syntax/README.md. I'd like to find a solution and remove them soon after this lands. Thanks again!

@stasm stasm merged commit e1eac00 into projectfluent:master Dec 13, 2018
@stasm stasm deleted the zeroeight branch December 13, 2018 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants