Skip to content

Bundle API and Localization class#136

Merged
Pike merged 4 commits intomasterfrom
next-gen-api
Apr 7, 2020
Merged

Bundle API and Localization class#136
Pike merged 4 commits intomasterfrom
next-gen-api

Conversation

@Pike
Copy link
Copy Markdown
Contributor

@Pike Pike commented Mar 16, 2020

This was an experimental branch to demonstrate how to implement the bundle API of other implementations, and to create a base Localization class for developers to use, and for binding-implementers to customize.

Per https://discourse.mozilla.org/t/experimental-release-of-python-fluent-runtime/47569/2, we should go ahead and merge this now.

CC @stasm @spookylukey

… bundles.

This change changes a lot of tests, as there's a shift from
format() to format_pattern(get_message().value).
With the get_message API, the complex lookups format('msg.attr') also expose
an API surface that's not inline with other impls, so I removed
quite a bit of utility code that served that purpose.
And as I changed all lines referencing bundles, I named the local
variable `bundle` instead of `ctx`.

There's a bit of clean-up sneaking in to this patch, triggered by
reshuffling the referencing code. Notably, all resolvers now
consistently return fluent types. In some cases, they returned
resolvers, which required resolving to get to types.
This corresponds to what @fluent/bundle does on the js side, but
doesn't try to be async at all.
@Pike Pike requested a review from spookylukey March 16, 2020 10:30
if not msg.value:
continue
val, errors = bundle.format_pattern(msg.value, args)
return val
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe return the errors, too?

return parser.parse(source)


class FluentBundle(object):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we move this to its own file?

self, locales, resource_ids, resource_loader,
use_isolating=False,
bundle_class=None, functions=None,
):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this PR the right place to comment on the design of the FluentLocalization API? Would you prefer to move this discussion to Discourse perhaps?

I'd like to suggest using the pattern we use in @fluent/react and @fluent/dom, i.e. rather than pass a lot of config parameters to the constructor, we pass an iterator over bundles created by the user. This allow the user of the API to be in charge of the exact bundle class used, the options and functions used to initialize it, and the loading logic.

@Pike
Copy link
Copy Markdown
Contributor Author

Pike commented Mar 16, 2020

The state of this branch is released and published on PYPI. It's on a branch in case it's a dead end, but it doesn't seem to be. I think we should merge this branch as is, and create follow-ups if we want to revise APIs or refactor parts.

For the expected behavior of the Localization class, discourse is probably a better platform than an implementation ticket, yeah.

@Pike Pike merged commit 7616c13 into master Apr 7, 2020
@Pike
Copy link
Copy Markdown
Contributor Author

Pike commented Apr 7, 2020

Per conversation on discourse and a chat with stasm on matrix, I went ahead and merged this.

@Pike Pike deleted the next-gen-api branch April 7, 2020 10:40
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