Skip to content

Alignment rules for linting and serialization #27

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

Open
stasm opened this issue Feb 3, 2017 · 4 comments
Open

Alignment rules for linting and serialization #27

stasm opened this issue Feb 3, 2017 · 4 comments
Labels
FUTURE Ideas and requests to consider after Fluent 1.0

Comments

@stasm
Copy link
Contributor

stasm commented Feb 3, 2017

In #24 (comment), @zbraniecki, @Pike, @flodolo and I had a discussion about the standard formatting and whitespace alignment of values in FTL.

The argument raised by @flodolo against pretty-aligning of values was that it breaks the blame and creates diff noise.

@zbraniecki then created an example file and expressed concerns about its readability without value-aligning:

0.1 syntax: https://github.com/projectfluent/fluent-rs/blob/0.1/tests/workload-low.ftl
0.2 syntax: https://github.com/projectfluent/fluent-rs/blob/master/tests/workload-low.ftl

To remind principles:
- the file should be readable without prior knowledge of FTL
- the file should be easy to scan through
- the file should be editable with minimal FTL knowledge

For one, I'd say we did manage to make it look cleaner (less sigils!) But also, I believe the attribute alignment significantly helps with readability, so I'm not sure if I agree that preserving blame is worth it.

I'm creating a new issue to give this topic its own place for discussion.

@stasm stasm changed the title Alignment rules for liniting erialization Alignment rules for liniting and serialization Feb 3, 2017
@stasm
Copy link
Contributor Author

stasm commented Feb 3, 2017

For comparison, here's the same file without any special alignment of values: https://gist.github.com/stasm/020a827c37004b44dbd61fd3a23817fe

@Pike
Copy link
Contributor

Pike commented Feb 3, 2017

Thanks for the example, I was looking for one. I finished up the brand.ftl part in https://gist.github.com/Pike/c1301c217e8fdb4246e55334049a5863#file-workload-low-ftl-L747

@stasm
Copy link
Contributor Author

stasm commented Feb 3, 2017

Thanks, I fixed my gist as well. (The vim macro that I used only removed the extra white space after the =.)

@Pike
Copy link
Contributor

Pike commented Feb 3, 2017

On a general note, I'm not a fan of putting linting rules into the implementation of serializers. It's mostly an even political opinion on mine, humans are right, machines are wrong. I somewhat know that there's a lot of linting in fashion today, but that might be something to implement at a different level than our core serializers.

On the examples here:

The example is a bit weird, in that the alignment rules are not coming from the IDs, but from the attributes. As such, the indention is determined by the grammar and not by the file. You'd only hit a change in indention across unrelated entries if you'd start using placeholder as attribute or something. Which is why I also put the brand part on the things to realign.

https://bugzilla.mozilla.org/attachment.cgi?id=8829980&action=diff#a/mobile/android/locales/en-US/mobile/aboutDownloads.ftl_sec2 might be an interesting other use-case.

Also, practically, we see alignment almost never on the values, but on the =, so

foo       = Foo

instead of

foo =       Foo

The latter is what the current examples show, and I find the unindented file easier to read. It might be just because of = being closer to the value.

Also, kinda specific to our chosen example, there's a tighter pattern of

some_id_thing
    .value = Value

and then the .accesskey stuff is a bit off in indention, which makes it almost easier to ignore.

@stasm stasm changed the title Alignment rules for liniting and serialization Alignment rules for linting and serialization Feb 13, 2017
@stasm stasm added this to the Syntax FUTURE milestone May 18, 2018
@stasm stasm added the FUTURE Ideas and requests to consider after Fluent 1.0 label May 23, 2018
@stasm stasm removed this from the Syntax FUTURE milestone May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FUTURE Ideas and requests to consider after Fluent 1.0
Projects
None yet
Development

No branches or pull requests

2 participants