Add a pre-commit configuration file and adopt Prettier #910
Conversation
|
Well, I've been procrastinating on this for a while. :) |
|
I'm gonna leave this up for a bit, to let folks chime in with thoughts on this. I'm assuming no one's gonna be strongly opposed, but I'm open to being surprised. :) |
|
FYI this has conflicts with these PRs:
(and probably others, I didn't check the rest.) They've been open quite a while at this stage (#796 in particular). Can they be actioned first? Adding additional conflicts to them is only going to add (potentially many) more months worth of iteration to them. I think an auto-formatter is probably best applied after all the other outstanding spec changes are dealt with, so it hits them all at once. |
|
I agree, let's finish the unfinished business first. Also, to avoid the noise, I'd suggest to configure the pretty printer so as to make its output similar to the current Markdown style used, in order to just remove inconsistencies rather than changing everything. In particular, this seems to mean:
Otherwise this looks good. |
Agreed.
In the long run, it would make sense to use the default Markdown style. One less thing to worry about. However, I think there's a way to configure Prettier with a TOML file. Correct me if I'm wrong, but the Prettier docs say that |
|
IMHO there is no such thing as a "default Markdown style", at least not when it comes to stuff such as However, while Prettier is configurable to some degree, I cannot see any options that control how Markdown output is formatted. Maybe I'm just overlooking something? If not, I'd suggest to look for another tool that's flexible enough to do a good job. This one, as far as I can tell, was mostly developed for JavaScript code, the Markdown support seems just a minor add-on and (I suspect) not all that well developed. |
|
I’ve no problem with having a standard style for markdown. I’d prefer @pradyunsg, as others have already mentioned, there are quite a few completed, verified, agreed upon PRs open. Before we merge this one in, those others should be merged. If there’s anything we can do to help to make this easier, please let us know (including adding new maintainers). |
This makes it easier to set up linters that check for various things within the repository.
This helps ensure that all the files within the repository have a consistent style.
This enforces that all Markdown files are wrapped at 80 characters, in the style the prettier follows.
11ce6f0 to
be51db4
Compare
Done. ;) #891 is gonna need more discussion, and spec language change -- so I don't wanna block on that.
As far as I am concerned, having an automated tool do the formatting for me is super useful, because -- as long as the output is understandable and "sane" -- it's very useful to have the tool enforce a consistent style across the project wherein no one needs to debate about style choices. The defaults of prettier are workable, with the line length enforcement being useful for easier-to-review diffs on the GitHub UI (which doesn't do too well with long lines, but highlights changed words). |
|
Might be nice to document what the max line length is now; it seems shorter than 80 based on this patch; it's now 79? 78? 76? Can maybe also add .editorconfig: |
|
It's 80, which is the default -- https://prettier.io/docs/en/options.html#print-width |
No worries, I’ll be able to deal with some reformatting there. |
This makes it easier to set up linters that check for various things
within the repository.