Add parameterized Attributes by stasm · Pull Request #212 · projectfluent/fluent · GitHub
Skip to content

Add parameterized Attributes#212

Merged
stasm merged 3 commits into
projectfluent:masterfrom
stasm:attr-params
Nov 14, 2018
Merged

Add parameterized Attributes#212
stasm merged 3 commits into
projectfluent:masterfrom
stasm:attr-params

Conversation

@stasm

@stasm stasm commented Nov 8, 2018

Copy link
Copy Markdown
Contributor

Fix #189. Add syntax to allow expressions of the form -term.attr(arg: "value"), e.g.:

-ship.gender(style: "chicago")
-echo.last-letter(count: 2)

@stasm stasm force-pushed the attr-params branch 2 times, most recently from 881d8fc to 0bbd3dd Compare November 14, 2018 12:05
@stasm stasm requested a review from Pike November 14, 2018 12:09
@stasm

stasm commented Nov 14, 2018

Copy link
Copy Markdown
Contributor Author

@Pike Pike left a comment

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.

r=me, this looks much better, thanks.

I think there's a left-over validation for AttributeExpression? Doesn't hurt, but I'd remove it.

Also, as a follow-up, do you want to update valid.md to the changes here, too?

Neither blocking.

Comment thread syntax/abstract.mjs

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.

I think this part is not needed with the reduced changes after the rebase? Attributes are always on messages or terms still.

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.

You're right, but I'd like to keep this here for the sake of completeness. I think it complements the validation of CallExpression. Let's keep it for now and I'll see if I can remove it in a follow-up.

@stasm stasm merged commit d011832 into projectfluent:master Nov 14, 2018
@stasm stasm deleted the attr-params branch November 14, 2018 16:00
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