Skip to content

Add private messages #130

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 3 commits into from
Jan 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fluent-syntax/src/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,10 @@ export class Identifier extends SyntaxNode {
}
}

export class Symbol extends Identifier {
export class VariantName extends Identifier {
constructor(name) {
super(name);
this.type = 'Symbol';
this.type = 'VariantName';
}
}

Expand Down
13 changes: 11 additions & 2 deletions fluent-syntax/src/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export class ParseError extends Error {
}
}

/* eslint-disable complexity */
function getErrorMessage(code, args) {
switch (code) {
case 'E0001':
Expand All @@ -32,9 +33,9 @@ function getErrorMessage(code, args) {
case 'E0007':
return 'Keyword cannot end with a whitespace';
case 'E0008':
return 'Callee has to be a simple identifier';
return 'The callee has to be a simple, upper-case identifier';
case 'E0009':
return 'Key has to be a simple identifier';
return 'The key has to be a simple identifier';
case 'E0010':
return 'Expected one of the variants to be marked as default (*)';
case 'E0011':
Expand All @@ -45,6 +46,14 @@ function getErrorMessage(code, args) {
return 'Expected literal';
case 'E0015':
return 'Only one variant can be marked as default (*)';
case 'E0016':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fluent-rs uses smaller errors numbers for these because it seems to be missing a few earlier ones.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to make error naming part of the developer productivity review after 0.7.

return 'Message references cannot be used as selectors';
case 'E0017':
return 'Variants cannot be used as selectors';
case 'E0018':
return 'Attributes of public messages cannot be used as selectors';
case 'E0019':
return 'Attributes of private messages cannot be used as placeables';
default:
return code;
}
Expand Down
49 changes: 36 additions & 13 deletions fluent-syntax/src/ftlstream.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,36 @@ export class FTLParserStream extends ParserStream {
return undefined;
}

isIDStart() {
if (this.ch === undefined) {
isCharIDStart(ch) {
if (ch === undefined) {
return false;
}

const cc = this.ch.charCodeAt(0);
return ((cc >= 97 && cc <= 122) || // a-z
(cc >= 65 && cc <= 90) || // A-Z
cc === 95); // _
const cc = ch.charCodeAt(0);
return (cc >= 97 && cc <= 122) || // a-z
(cc >= 65 && cc <= 90); // A-Z
}

isMessageIDStart() {
if (this.currentIs('-')) {
this.peek();
}

const ch = this.currentPeek();
const isID = this.isCharIDStart(ch);
this.resetPeek();
return isID;
}

isNumberStart() {
const cc = this.ch.charCodeAt(0);
return ((cc >= 48 && cc <= 57) || cc === 45); // 0-9
if (this.currentIs('-')) {
this.peek();
}

const cc = this.currentPeek().charCodeAt(0);
const isDigit = cc >= 48 && cc <= 57; // 0-9
this.resetPeek();
return isDigit;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the value of this change?

Copy link
Contributor Author

@stasm stasm Jan 22, 2018

Choose a reason for hiding this comment

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

It makes isNumberStart consistent with isMessageIDStart and more importantly, it considers the char after the leading -. The previous version would return true upon seeing a dash right away and more checking was required in getLiteral.

}

isPeekNextLineZeroFourStyleComment() {
Expand Down Expand Up @@ -252,7 +268,7 @@ export class FTLParserStream extends ParserStream {
while (this.ch) {
if (this.currentIs('\n') && !this.peekCharIs('\n')) {
this.next();
if (this.ch === undefined || this.isIDStart() ||
if (this.ch === undefined || this.isMessageIDStart() ||
(this.currentIs('/') && this.peekCharIs('/')) ||
(this.currentIs('[') && this.peekCharIs('['))) {
break;
Expand All @@ -262,13 +278,20 @@ export class FTLParserStream extends ParserStream {
}
}

takeIDStart() {
if (this.isIDStart()) {
takeIDStart(allowPrivate) {
if (allowPrivate && this.currentIs('-')) {
this.next();
return '-';
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be able to return this.next(); here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to return current here but first, we advance the index.


if (this.isCharIDStart(this.ch)) {
const ret = this.ch;
this.next();
return ret;
}
throw new ParseError('E0004', 'a-zA-Z_');

const allowedRange = allowPrivate ? 'a-zA-Z-' : 'a-zA-Z';
throw new ParseError('E0004', allowedRange);
}

takeIDChar() {
Expand All @@ -283,7 +306,7 @@ export class FTLParserStream extends ParserStream {
return this.takeChar(closure);
}

takeSymbChar() {
takeVariantNameChar() {
const closure = ch => {
const cc = ch.charCodeAt(0);
return ((cc >= 97 && cc <= 122) || // a-z
Expand Down
97 changes: 65 additions & 32 deletions fluent-syntax/src/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export default class FluentParser {

// Poor man's decorators.
[
'getComment', 'getMessage', 'getAttribute',
'getIdentifier', 'getVariant', 'getSymbol', 'getNumber', 'getPattern',
'getComment', 'getMessage', 'getAttribute', 'getIdentifier',
'getVariant', 'getVariantName', 'getNumber', 'getPattern',
'getTextElement', 'getPlaceable', 'getExpression',
'getSelectorExpression', 'getCallArg', 'getString', 'getLiteral',
].forEach(
Expand Down Expand Up @@ -140,7 +140,7 @@ export default class FluentParser {
return null;
}

if (ps.isIDStart() && (!comment || comment.type === 'Comment')) {
if (ps.isMessageIDStart() && (!comment || comment.type === 'Comment')) {
return this.getMessage(ps, comment);
}

Expand Down Expand Up @@ -239,7 +239,7 @@ export default class FluentParser {

ps.skipInlineWS();

this.getSymbol(ps);
this.getVariantName(ps);

ps.skipInlineWS();

Expand All @@ -251,7 +251,7 @@ export default class FluentParser {
}

getMessage(ps, comment) {
const id = this.getIdentifier(ps);
const id = this.getPrivateIdentifier(ps);

ps.skipInlineWS();

Expand Down Expand Up @@ -280,7 +280,7 @@ export default class FluentParser {
getAttribute(ps) {
ps.expectChar('.');

const key = this.getIdentifier(ps);
const key = this.getPublicIdentifier(ps);

ps.skipInlineWS();
ps.expectChar('=');
Expand Down Expand Up @@ -311,10 +311,18 @@ export default class FluentParser {
return attrs;
}

getIdentifier(ps) {
getPrivateIdentifier(ps) {
return this.getIdentifier(ps, true);
}

getPublicIdentifier(ps) {
return this.getIdentifier(ps, false);
}

getIdentifier(ps, allowPrivate) {
let name = '';

name += ps.takeIDStart();
name += ps.takeIDStart(allowPrivate);

let ch;
while ((ch = ps.takeIDChar())) {
Expand All @@ -337,7 +345,7 @@ export default class FluentParser {
return this.getNumber(ps);
}

return this.getSymbol(ps);
return this.getVariantName(ps);
}

getVariant(ps, hasDefault) {
Expand Down Expand Up @@ -396,21 +404,21 @@ export default class FluentParser {
return variants;
}

getSymbol(ps) {
getVariantName(ps) {
let name = '';

name += ps.takeIDStart();
name += ps.takeIDStart(false);

while (true) {
const ch = ps.takeSymbChar();
const ch = ps.takeVariantNameChar();
if (ch) {
name += ch;
} else {
break;
}
}

return new AST.Symbol(name.trimRight());
return new AST.VariantName(name.trimRight());
}

getDigits(ps) {
Expand Down Expand Up @@ -544,25 +552,42 @@ export default class FluentParser {

if (ps.currentIs('-')) {
ps.peek();

if (!ps.currentPeekIs('>')) {
ps.resetPeek();
} else {
ps.next();
ps.next();
return selector;
}

ps.skipInlineWS();
if (selector.type === 'MessageReference') {
throw new ParseError('E0016');
}

const variants = this.getVariants(ps);
if (selector.type === 'AttributeExpression' &&
!selector.id.name.startsWith('-')) {
throw new ParseError('E0018');
}

if (selector.type === 'VariantExpression') {
throw new ParseError('E0017');
}

if (variants.length === 0) {
throw new ParseError('E0011');
}
ps.next();
ps.next();

ps.expectIndent();
ps.skipInlineWS();

const variants = this.getVariants(ps);

return new AST.SelectExpression(selector, variants);
if (variants.length === 0) {
throw new ParseError('E0011');
}

ps.expectIndent();

return new AST.SelectExpression(selector, variants);
} else if (selector.type === 'AttributeExpression' &&
selector.id.name.startsWith('-')) {
throw new ParseError('E0019');
}

return selector;
Expand All @@ -580,7 +605,7 @@ export default class FluentParser {
if (ch === '.') {
ps.next();

const attr = this.getIdentifier(ps);
const attr = this.getPublicIdentifier(ps);
return new AST.AttributeExpression(literal.id, attr);
}

Expand All @@ -601,7 +626,7 @@ export default class FluentParser {

ps.expectChar(')');

if (!/^[A-Z_-]+$/.test(literal.id.name)) {
if (!/^[A-Z][A-Z_?-]*$/.test(literal.id.name)) {
throw new ParseError('E0008');
}

Expand Down Expand Up @@ -693,17 +718,25 @@ export default class FluentParser {
throw new ParseError('E0014');
}

if (ch === '$') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the $ check to the top as this will likely be the most common literal.

ps.next();
const name = this.getPublicIdentifier(ps);
return new AST.ExternalArgument(name);
}

if (ps.isMessageIDStart()) {
const name = this.getPrivateIdentifier(ps);
return new AST.MessageReference(name);
}

if (ps.isNumberStart()) {
return this.getNumber(ps);
} else if (ch === '"') {
}

if (ch === '"') {
return this.getString(ps);
} else if (ch === '$') {
ps.next();
const name = this.getIdentifier(ps);
return new AST.ExternalArgument(name);
}

const name = this.getIdentifier(ps);
return new AST.MessageReference(name);
throw new ParseError('E0014');
}
}
12 changes: 6 additions & 6 deletions fluent-syntax/src/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default class FluentSerializer {
}

for (const entry of resource.body) {
if (entry.types !== 'Junk' || this.withJunk) {
if (entry.type !== 'Junk' || this.withJunk) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A nasty typo but fortunately didn't impact many tests. It made the serializer serialize Junks verbatim as they appear in the parsed text. Input magically always matched the output...

parts.push(this.serializeEntry(entry));
if (this.hasEntries === false) {
this.hasEntries = true;
Expand Down Expand Up @@ -88,7 +88,7 @@ function serializeResourceComment(comment) {
}

function serializeSection(section) {
const name = serializeSymbol(section.name);
const name = serializeVariantName(section.name);

if (section.comment) {
const comment = serializeComment(section.comment);
Expand Down Expand Up @@ -309,15 +309,15 @@ function serializeIdentifier(identifier) {
}


function serializeSymbol(symbol) {
return symbol.name;
function serializeVariantName(VariantName) {
return VariantName.name;
}


function serializeVariantKey(key) {
switch (key.type) {
case 'Symbol':
return serializeSymbol(key);
case 'VariantName':
return serializeVariantName(key);
case 'NumberExpression':
return serializeNumberExpression(key);
default:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
key = { foo.23 }

//~ ERROR E0004, pos 12, args "a-zA-Z_"
//~ ERROR E0004, pos 12, args "a-zA-Z"

key = { foo. }

//~ ERROR E0004, pos 31, args "a-zA-Z_"
//~ ERROR E0004, pos 31, args "a-zA-Z"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
err1 = { -brand.gender }
//~ ERROR E0019, pos 23
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
err1 =
{ foo.bar ->
[1] One
*[2] Two
}
//~ ERROR E0018, pos 21
Loading