Add prettier

[phpnode]

Prettier makes it easier to maintain consistency in the codebase, especially when there's several different contributors. I'd like to enable it, but it's not as easy as it would normally be because several tests use formatting to convey intent and to make things more readable. We should either refactor those tests to convey similar intent in a way that prettier won't ruin, or we should use some prettier ignore comments.

[hildjj]

I'm open to this, but I think I'd also like to do the code-generation refactor first (see #44), because the current state of play in generate-js.js is scary, from an indentation perspective.

[StoneCypher]

fully 10% of the prs that go south at work are because someone had to spend an hour fighting a formatter that wasn't able to understand that it was breaking things

[hildjj]

I'm fine with the current approach. If anyone feels strongly about prettier, please send a PR.

[markw65]

I'm fine with the current approach. If anyone feels strongly about prettier, please send a PR.

Is this still the case? I'll put up a PR if you're still willing to take it...

[hildjj]

I'd really rather not change all the formatting at this point, but if you can make prettier get "close enough" to what we currently have, I guess I'd look at it. Let's land 425 and 427 first though, please.

[markw65]

Also, if the answer is yes, is it ok to change some of the eslint rules to match prettier's behavior?

eg prettier has no option to split lines before the operator, so the eslint rule "operator-linebreak": "before" complains.

[hildjj]

I really don't want to change the eslint rules unless we get enough benefit out of it. I really don't see prettier as a benefit, since I have my editor configured to give visual cues for every lint violation, and I fix them as I go.

[markw65]

Actually, while prettier is my preferred formatter, I'm not really attached to it. What I really like about it is that I've configured it in my own projects to format on save, so I no longer have to worry about where to insert spaces, or break lines etc. I just type and save.

But I've had to disable that in peggy to avoid breaking all the lint rules (and generally reformatting code that I'm not working on) - which has meant that I have to painstakingly fix all the nits that eslint throws at me.

So all I really want is something that will fix the eslint nits automatically - and it seems that I can just switch to using eslint as a formatter, and get essentially the behavior I'm looking for (not quite as good as prettier - it won't auto fix all the issues that it complains about, but seems to deal with most of them).

So at this point, I'm mostly happy. Would a PR for some VSCode settings be acceptable? It would add a .vscode directory with a couple of files in it at the top level.

[markw65]

since I have my editor configured to give visual cues for every lint violation, and I fix them as I go.

I have that too. But compared with being able to just ignore them, hit save, and have them auto-fixed its painful. But as I said, configuring eslint as the formatter seems to solve that issue (in VSCode at least).

[hildjj]

I would accept a patch that adds .vscode/ to git .gitigore file. I just realized that I've got it in my ~/.gitignore_global file.

[markw65]

I would accept a patch that adds .vscode/ to git .gitigore file

I guess I see why you wouldn't want the .vscode/settings.json file checked in, even if it only did uncontroversial (I think!) things like setting the default formatter. Because now users can't configure /other/ project specific things that they might want to.

But what about this: there could be a checked in peggy.code-workspace file which sets eslint as the default formatter (and any other vscode settings that were deemed project specific could also be added there). Then if you open peggy via peggy.code-workspace you get whatever default settings are presented there, but the (uncommitted) ./vscode/settings.json would take precedence over it (and you could still choose to just open the peggy folder anyway).

Also I think it would still be useful to add a .vscode/extensions.json file with dbaeumer.vscode-eslint and peggyjs.peggy-language as recommended plugins.

[hildjj]

That sounds like a workable plan. peggyjs.peggy-language has a circular dependency on peggy, so when adding new features to the examples, we'll see a few issues. I think I'm willing to live with those, but we can turn it off if it's a problem.

note to self: adding @peggyjs/eslint-plugin as a circular dependency would be MUCH worse.

[markw65]

circular dependency on peggy, so when adding new features to the examples, we'll see a few issues

Note that extensions.json just provides a list of recommended extensions; it doesn't force you to install or enable them (but it does give you a one click way to do so).

[hildjj]

Fixed in #432.