Skip to content
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

Require = after the identifier #63

Closed
1 of 3 tasks
stasm opened this issue Nov 3, 2017 · 20 comments
Closed
1 of 3 tasks

Require = after the identifier #63

stasm opened this issue Nov 3, 2017 · 20 comments
Milestone

Comments

@stasm
Copy link
Contributor

stasm commented Nov 3, 2017

Right now the equals sing following the identifier = is part of the value production in the EBNF. Message without values are thus written like this:

foo-bar
    .attr = Attribute

I would like to change this to always require the = after the identifier. This would clearly demarcate the identifier and the indented body of the message, similar to Python's :.

foo-bar =
    .attr = Attribute

In the rare cases when the message should have an empty string for its value, the same explicit syntax as in #32 can be used:

foo-bar = {""}
    .attr = Attribute

Signoffs

@Pike
Copy link
Contributor

Pike commented Nov 3, 2017

I'm not a big fan of this.

Also the value would be different in foo and bar, right?

foo = 

bar = 
    .attr = shiney

@stasm
Copy link
Contributor Author

stasm commented Nov 3, 2017

foo in your example would be a message without a value and any attributes, which right now is a syntax error.

@stasm stasm added this to the Syntax 0.5 milestone Nov 9, 2017
@zbraniecki
Copy link
Collaborator

I think of the role of = differently and would prefer to see:

foo
    .label = Label
    .accesskey = R

but I believe that it's forward compatible to require it now and drop this requirement later, so if stas is committed to this change, I'm not going to block. I'd just like to see an issue filed for syntactic sugar of dropping it later ;)

@stasm stasm added the syntax label Nov 10, 2017
@hkasemir
Copy link
Contributor

I don't have a strong opinion on this, I could see it making a lot of sense either way, but aesthetically I somewhat prefer without the =

foo-bar
    .attr = Attribute

@zbraniecki
Copy link
Collaborator

@stasm gathering more feedback on 0.5 changes this one stands out as something users don't see a value in. Is there a chance we could consider the syntax sugar to become available in 0.5 alongside this change?

@stasm
Copy link
Contributor Author

stasm commented Dec 1, 2017

I'll think about it. I'd prefer to start with a single approach but I'd like to consider the wider context. I'll update this issue in a few days.

@stasm
Copy link
Contributor Author

stasm commented Dec 1, 2017

I filed #71 to discuss the syntax sugar.

I found a few more reasons in favor of requiring = after the identifier.

  • It makes the = part of the message definition rather than part of the value. It's consistent with the fact that variant values don't start with = either and helps communicate the fact that the message has an empty (null) value.

  • It helps scanning bigger files for languages using the Latin alphabet. Take a glance at fluent.js's benchmark file. Thanks to the indentation it's easy to find the beginning of each message. However, without the = at the end of the line with the identifier, it's hard to pick up messages with null values. The end of the identifier consists of Latin letters and looks like it might be the end of the translation value.

    open-file-menuitem # ← This could be the end of the translation value.
        .label = Open File…
        .accesskey = O
  • (Minor, but nice.) We can leverage syntax highlighting for properties even further. In the snippet right above which doesn't have the =, the open-file-menuitem identifier is not highlighted.

@stasm
Copy link
Contributor Author

stasm commented Dec 4, 2017

In #71 (comment), @flodolo wrote:

foo-bar =
    .attr = Attribute Value

I find this extremely confusing. I think that anyone reading the file should be able to mentally strip whitespaces, and still make sense of the syntax. This one reads like:

foo-bar = .attr = Attribute Value

Where the value of foo-bar is .attr = Attribute Value.

Wouldn't the same be true for the simple case of:

foo = Foo Value
    .attr = Foo Attribute

Does this read like the value of foo is Foo Value .attr = Foo Attribute?

@stasm
Copy link
Contributor Author

stasm commented Dec 4, 2017

In #71 (comment), @zbraniecki wrote:

The = sign after the identifier with no value is not helping with readability
In fact, it does the opposite, it makes it harder to understand that the given message has no value

I acknowledge that "you believe" these things, but I don't know how to address them without understanding the reasons. Are they purely subjective? What do you think about the second point from my comment #63 (comment) about mistaking the end of the identifier for the end of the translation?

It removes the way to distinguish empty value from lack of value

You can do the following which is consistent with how we treat leading whitespace:

# The value is "    Foo".
foo = {"    "}Foo

# The value is an empty string.
bar = {""}
    .attr = Bar Attribute

@flodolo
Copy link
Contributor

flodolo commented Dec 4, 2017

Does this read like the value of foo is Foo Value .attr = Foo Attribute?

No, because there's a clear visual separation between the value, and the attribute declaration (and yes, I'm weirdly aware that contradicts the "stripping whitespaces" comment).

For example, in this case I'd consider .attr to be a strange string ID with value Bar Attribute, and I'd need to dig into the syntax to understand that an ID can't start with .

foo = Foo Value
.attr = Foo Attribute

@Pike
Copy link
Contributor

Pike commented Dec 4, 2017

I'd like to put this on the talking points for tomorrow's tech call.

Note, I've removed my tick because I'm not sure what it should mean, and if my tick meant that.

At this point I also don't see a tick by gandalf, but one of my tabs says it had one. Can't find the history of the tick marks for the love of it.

@stasm
Copy link
Contributor Author

stasm commented Dec 4, 2017

I'd like to put this on the talking points for tomorrow's tech call.

Can we reschedule this? I'll be traveling and won't be joining the calls tomorrow.

Note, I've removed my tick because I'm not sure what it should mean, and if my tick meant that.

I'd like the tick to say: I'm OK with this becoming part of Fluent.

At this point I also don't see a tick by gandalf, but one of my tabs says it had one. Can't find the history of the tick marks for the love of it.

Yes, I remember that at one point this issue had 3 ticks :) That's why when I proposed the signoff process in #62 (comment) I also suggested that we explicitly comment with an r+.

@zbraniecki
Copy link
Collaborator

zbraniecki commented Jan 15, 2018

It's quite scary and amusing how closely we follow Walder's Law

Can't wait to get to #16 :)

@flodolo
Copy link
Contributor

flodolo commented Jan 18, 2018

At risk of pointing out the obvious: we have 4 FTL strings in Firefox 59, two of them fall into this bucket, a lot more in the other patch for Preferences chrome.

What happens to 59 if we introduce = for empty values? Will we need to uplift the new version to Beta?

@stasm
Copy link
Contributor Author

stasm commented Jan 18, 2018

At risk of pointing out the obvious: we have 4 FTL strings in Firefox 59, two of them fall into this bucket, a lot more in the other patch for Preferences chrome.

For the existing strings, I've researched the compatibility of MessageContext shipped in Firefox 59 in bug 1426463. The conclusion is:

It turns out we don't have to do anything. For comments, the parser doesn't know how to parse the # comments, so it emits an error and resumes parsing at the next valid identifier. It looks like our error recovery saved the day. For the trailing = followed by an attribute, the error I'm seeing in the Playground is a regression introduced in fluent 0.4.2. And since MessageContext.jsm is based on 0.4.1, it Just Works™.

I'd like for us to update the Preferences patch (bug 1424682) to the new syntax before we land it.

@flodolo
Copy link
Contributor

flodolo commented Jan 18, 2018

For the existing strings, I've researched the compatibility of MessageContext shipped in Firefox 59 in bug 1426463.

Thanks, I missed that was covered as well (I assumed it was only comments)

@zbraniecki
Copy link
Collaborator

I'd like for us to update the Preferences patch (bug 1424682) to the new syntax before we land it.

Good point! Updated.

@Pike
Copy link
Contributor

Pike commented Jan 18, 2018

The conclusion is:

It turns out we don't have to do anything. For comments, the parser doesn't know how to parse the # comments, so it emits an error and resumes parsing at the next valid identifier. It looks like our error recovery saved the day. For the trailing = followed by an attribute, the error I'm seeing in the Playground is a regression introduced in fluent 0.4.2. And since MessageContext.jsm is based on 0.4.1, it Just Works™.

That conclusion only works for the comment sigils, but not for the required equals sign in browser/preferences/main.ftl.

@stasm
Copy link
Contributor Author

stasm commented Jan 18, 2018

Let's move this to bug 1426463 and keep this issue about the proper change in the Syntax, please.

@stasm
Copy link
Contributor Author

stasm commented Jan 30, 2018

I made the call to go ahead with this proposal. The = will be required after the message identifier in Syntax 0.5. Thanks to everyone who participated in the discussion and who offered their feedback.

There were valid arguments for and against the proposal. In the end I was convinced by how the required = sign improves the learnability (for the lack of a better word) of the syntax.

When someone not acquainted with the syntax looks at an FTL file for the first time, they look for patterns which will help them discover the rules governing the syntax. The more coherent the syntax is, the easier it is to discover these rules.

The = after the identifier is one such pattern. It helps figure out the rule about how messages are defined just by looking at an FTL file. OTOH, identifiers without = may look like they define something else rather than messages.

For the same reason of discoverability, going forward I will be likely against introducing any syntax sugar into the Fluent Syntax. Syntax sugar can save a few key strokes but for someone not familiar with the syntax it creates cognitive burden—it looks like something new which needs to be understood and learned independently. It's the simple vs. easy dilemma. Syntax sugar is easy to use once you know the underlying concepts, but it makes the syntax less simple by introducing complexity and choice.

Explicit syntax without shortcuts improves the discoverability. In fact, as @jakobkappel suggested in https://gist.github.com/flodolo/164ae39703d5479d799954e4b0da8f76, the most readable and discoverable syntax would explicitly define the null value. Something like:

message-key = { NONE }
    .attr = Attribute Value

Or without the need for a reserved keyword like NONE:

message-key = {}
    .attr = Attribute Value

These suggestions are well in line with my intent to improve the discoverability of the syntax. If the user feedback about Syntax 0.5 and the required = makes us revisit this decision, I'd like to explore these more explicit options further.

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

No branches or pull requests

5 participants