Skip to content

FluentBundle.compound#360

Closed
stasm wants to merge 6 commits intoprojectfluent:masterfrom
stasm:compound
Closed

FluentBundle.compound#360
stasm wants to merge 6 commits intoprojectfluent:masterfrom
stasm:compound

Conversation

@stasm
Copy link
Copy Markdown
Contributor

@stasm stasm commented Apr 26, 2019

This is a continuation of the work started by @unclenachoduh in #322.

@stasm stasm mentioned this pull request Apr 26, 2019
get messages() {
return this._messages[Symbol.iterator]();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be good to retain some way of iterating over a bundle's messages. Atm. e.g. print() in tools/format.js ends up needing to access bundle._messages.

How about something like this?

get messageKeys() {
  return this._messages.keys();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need to add this right now. I don't know of good runtime use-cases for iterating over all messages in a bundle. I usually prefer to keep the API surface as small as possible, and extend it only when needed.

The code in tools/format.js should use the result of the full-AST parsing from fluent-syntax to build an iterable of message keys for the purpose of printing all messages. I intend to fix it in a follow-up but I can also do it here.

@stasm stasm mentioned this pull request Apr 29, 2019
if (typeof message === "string") {
return this._transform(message);
format(path, args, errors) {
let parts = path.split(".");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you could just use path.split(".", 1); to separate one . and not bother with anything past that. The rest is an attribute name.

@stasm
Copy link
Copy Markdown
Contributor Author

stasm commented Jul 11, 2019

We're going to the go with the formatPattern proposal in #380.

@stasm stasm closed this Jul 11, 2019
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.

4 participants