Require message body to be indented by stasm · Pull Request #32 · projectfluent/fluent · GitHub
Skip to content

Require message body to be indented#32

Merged
stasm merged 1 commit into
projectfluent:masterfrom
stasm:indent
Feb 24, 2017
Merged

Require message body to be indented#32
stasm merged 1 commit into
projectfluent:masterfrom
stasm:indent

Conversation

@stasm

@stasm stasm commented Feb 15, 2017

Copy link
Copy Markdown
Contributor

Fix #12, #17, #18.

With this change, the entire body of a message needs to indented. This makes
error recovery very easy: finding the next message definition is as simple as
finding the next identifier with no indentation.

It also opens up a number of opportunities: we can remove the | syntax for
multiline blocks of text and allow line breaks inside of placeables safely.

The PR also allows the value to be defined on a new line, making the
following examples equivalent:

lipsum = Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi
    pellentesque congue metus, non mattis sem faucibus sit amet.

lipsum
    = Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi
    pellentesque congue metus, non mattis sem faucibus sit amet.

I hope this will help when attributes are present:

lipsum
    = Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi
    pellentesque congue metus, non mattis sem faucibus sit amet.

    .attr = Attribute

Lastly, quoted patterns are only available inside of placeables and cannot be
used directly as values.

The exact semantics of \ escapes will be defined in #22.

@stasm

stasm commented Feb 15, 2017

Copy link
Copy Markdown
Contributor Author

@zbraniecki

Copy link
Copy Markdown
Collaborator

Lastly, quoted patterns are only available inside of placeables and cannot be
used directly as values.

Does it remove the ability to use quoted patterns in order to get significant trailing/leading white spaces?

@stasm

stasm commented Feb 15, 2017

Copy link
Copy Markdown
Contributor Author

Does it remove the ability to use quoted patterns in order to get significant trailing/leading white spaces?

Just a little bit. In #22 I'd like to define all backslash-escapes and allow escaping leading and trailing whitespace either explicitly or through Unicode escapes.

Do you think it's a common enough use-case that we should give it better support?

@zbraniecki

Copy link
Copy Markdown
Collaborator

Idk, we can ask @Flod. But I know that from the readability principle perspective:

key = "  Value"

is a very clear way to say - I want key to have this value and those two whitespaces in front are meaningful.
Using unicode escapes or backslashes to annotate the same is a departure from that principle.

@Pike

Pike commented Feb 15, 2017

Copy link
Copy Markdown
Contributor

I'll look into the actual diff here in more detail, but regarding the leading/trailing whitespace, just one \ should suffice?

key = \     Value

would easily denote " Value".

Trailing whitespace is more tricky, as it's hard to distinguish "" from "\ ".

That said, on this particular aspect, my experience is that we rarely see leading or trailing whitespace in strings. In particular, if we would, we might want to make whitespace around = significant, wouldn't we?

@zbraniecki

Copy link
Copy Markdown
Collaborator

is there any particular reason to remove the quote-delimited strings?

If it's just for simplicity, we could ensure we don't lock ourselves out of this solution and keep it in a sleeve in case a use case pops up later.

@stasm

stasm commented Feb 15, 2017

Copy link
Copy Markdown
Contributor Author

I would be OK forbidding quoted strings in placeables: { "foo" } as well as changing the syntax of named-argument to only allow a single unquoted word as value.

@flodolo

flodolo commented Feb 16, 2017

Copy link
Copy Markdown
Contributor

While I agree that key = " Value" is really clear, it's also a bit confusing as a localizer looking directly at the FTL file.

First thought when put in front of this: are these quotes going to show up in the product? Should I replace these with prettier/proper double quotes?

If the expectation is for localizers to work in tools, and for those who don't to be smart enough to be familiar with the syntax, I think that's a good choice.

Escaping the leading whitespace seems more confusing to me. Also reminds me of multiline properties, even if it's leading and not trailing.

P.S. sadly flod was taken when I signed up to GH, so you pinged a random inactive user, but I'm watching this project these days

@Pike

Pike commented Feb 16, 2017

Copy link
Copy Markdown
Contributor

On the actual diff, there are a few edgecases still.

AFAICT, there can be one empty line, but only one at a time in a message body?

key = value

  .attr = attribute

is ok, but

key  = value


 .attr = attribute

isn't?

Also, there are a few cases where EOF is probably OK instead of NL?

I personally would also like to see

key = 
    Some lengthy content here that doesn't have a = at the
    beginning.

While thinking of EOF, I also noticed that the grammar doesn't allow for leading whitespace-only lines.

@stasm stasm requested review from Pike and zbraniecki February 23, 2017 16:18
@stasm stasm mentioned this pull request Feb 23, 2017
@stasm

stasm commented Feb 23, 2017

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.

As I sat next to stas while he typed this, I'm in favor of this in principle, and didn't find anything to nag about on the details of the grammar.

Fix projectfluent#12, projectfluent#17, projectfluent#18.

With this change, the entire body of a message must be indented. This makes
error recovery very easy: finding the next message definition is as simple as
finding the next identifier with no indentation.

It also opens up a number of opportunities: we can remove the `|` syntax for
multiline blocks of text and allow line breaks inside of placeables safely.

The change also allows the value to be defined on a new line, making the
following examples equivalent:

    lipsum = Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi
        pellentesque congue metus, non mattis sem faucibus sit amet.

    lipsum =
        Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi
        pellentesque congue metus, non mattis sem faucibus sit amet.

Lastly, quoted patterns are only available inside of placeables, cannot contain
aother placeables and cannot be used directly as values.

The exact semantics of \ escapes will be defined in projectfluent#22.
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.

4 participants