Skip to content

Implement text-mode-first syntax #500

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 16 commits into from
Oct 26, 2023
Merged

Conversation

aphillips
Copy link
Member

@aphillips aphillips commented Oct 23, 2023

Following the 2023-10-23 teleconference, implement "option 2a with additional ugliness".

Following the 2023-10-23 teleconference, implement "option 2a with additional ugliness".

(This PR is not yet ready for review. It will include updating the syntax.md.)
aphillips and others added 2 commits October 23, 2023 13:23
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
@aphillips aphillips requested a review from eemeli October 23, 2023 20:25
Copy link
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

I think that might be everything? So looks good.

For anyone coming to this without context and wondering what the **** we're thinking with a syntax that sometimes ends with }}}}}:

We agreed on today's call to resolve the syntax blockage we'd gotten ourselves in by picking a suitably ugly version of one of the variants under consideration, so that we'd have a baseline that at least nominally fulfills our preceding consensus decisions on not delimiting simple patterns and using {{...}} for complex patterns.

This is not intended as the final MF2 syntax, but as a step towards that. It will be iterated on.

With this to work from, we're now focusing on revisiting our consensus on whether to require non-simple patterns to always be quoted, or to make that optional.

@aphillips
Copy link
Member Author

@echeran @mihnita Can I get one of you to approve this before I merge it?

Copy link
Collaborator

@echeran echeran left a comment

Choose a reason for hiding this comment

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

@mihnita Can you double check the .properties file examples for correctness?

Apart from that, everything else LGTM. Hopefully, we're all satisfied in the ugliness that has been achieved.

> hello = { Hello }
> hello2={ Hello }
> hello = {{{{ Hello }}}}
> hello2=\ Hello \
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems off, too, IIUC (.properties format doc)

Based on what we had before, it seemed the only difference was the whitespace around the = sign, which the start-in-code-mode syntax was resilient against. With this PR, we can only allow variable whitespace on the left side of the equals sign.

Suggested change
> hello2=\ Hello \
> hello2= Hello

Copy link
Member Author

@aphillips aphillips Oct 26, 2023

Choose a reason for hiding this comment

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

LOL. I already had a hello2 I guess...

The space trimming here is an artifact of .properties, not of MF2. The problem is to get intentional spaces to the MF2 formatter. The hello2 example shows how to do that in a properties file (a problem that already exists for users of this format--look for it on stackoverflow, for example).

From an MF2 perspective, Hello is perfectly valid and clear about space handling. We don't need to solve every format's way of protecting the spaces in order for MF2 to be done. As you point out, the old syntax did guard against that... at the cost of making the message Hello invalid.

One might ask the question: why would you need to pass Hello through MF2, since it has no replacement variables (MF2 doesn't do anything to it). One reason might be because the source string has a replacement that the translator has removed. An example of this might be a gender inflected language. The source string Welcome {$user} in Czech needs to account for the user's gender:

Vítejte Tim
Vítej Tanyo

If there is no gender data available, the Czech translator might translate the message to omit the personal greeting. So:

welcome.properties:

hello = Welcome {$user}!

welcome-cs.properties:

hello = Vítejte uživateli!

This string would pass through MF2 because the caller doesn't know whether it has a replacement or not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a good point. I'll leave it unresolved so that I have a better chance of finding it again later. It SGTM. (And .properties is still weird (and still popular).)

@aphillips aphillips merged commit 8d58f80 into main Oct 26, 2023
@aphillips aphillips deleted the aphillips-text-mode-first-syntax branch November 28, 2023 16:13
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.

5 participants