Store unescaped content in StringLiteral.value and raw content in StringLiteral.raw

[stasm]

This also forbids escape sequences representing surrogate code points, following @Pike's suggestion from #194 (comment). If we allow them, then their handling becomes environment/implementation dependent. E.g. they are ill-formed in the UTF-8 encoding.

[Pike]

I'm using .val and .raw_val in compare-locales, and I'm very unhappy about that. Just yesterday had to change one for the other, because the difference isn't obvious.

That said, maybe that's OK within this context, but we'll need comments for that.

Also, please keep the current semantics of .value, as most tools use that and will want to continue to use that.
How about .format_value or .bundle_value for the unescaped value? That indicates the API in which to use that variant?

[stasm]

That said, maybe that's OK within this context, but we'll need comments for that.

Where would you put it?

Also, please keep the current semantics of .value, as most tools use that and will want to continue to use that.

Tools will need to be updated anyways for Syntax 08, right? That's not a good reason for not making this change.

I prefer to call out the special-ness of the raw value field, which is why I called it raw. And then value is the unescaped one. That's the naming everyone in #195 have been using and also what I've seen in many other ASTs.

If you feel very strongly about this, I guess we could keep value as the raw one, and add unescaped for the unescaped one.

[Pike]

Where would you put it?

In ast.mjs, so that consumers of that API know what to use when.

[Pike]

This is technically OK, but I disagree with the choice out of my own experience in compare-locales.

Practically, this is going to add a significant risk in particular to the Pontoon support, as we're changing the data the existing .value API returns.

Not going to block this, but also not going to r+ this, so removing myself from the review.

[Pike]

Meh, can't find a way to actually remove myself without showing my previous r-.

[stasm]

Practically, this is going to add a significant risk in particular to the Pontoon support, as we're changing the data the existing .value API returns.

Thank you for you comment. I think I'm misunderstanding something here. When we update Pontoon to Syntax 0.8, we'll change the code which deals with StringLiterals to use the .raw field rather than .value. It sounds like this will solve this problem completely. We'll need to make changes to tools anyways. Is there more to this which I'm not seeing?

[stasm]

Actually, it looks like updating the serializer to use .raw for StringLiterals (which we'll need to do anyways) will be enough for Pontoon: https://github.com/mozilla/pontoon/blob/d55d260554d13173e4ee39629becbd0ab3a264af/pontoon/base/static/js/fluent_interface.js#L435.

[Pike]

My point is that I made the same exact API decision, and it's been a source of bugs (just this week, actually).

That said, I don't want to spend more time on this ticket.

[stasm]

OK, thanks again. I'm going to merge this as-is.