chore: enable prettier for markdown files and apply to all files by virkt25 · Pull Request #1139 · loopbackio/loopback-next

virkt25 · 2018-03-15T16:33:43Z

Changes are broken down into 3 commits.

Add prettier config for markdown scripts (supports GitHub Flavoured Markdown)
Run prettier with new config to fix all files.
Update main README to use reference style for GitHub User profile images

Connected to #1099

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
Related API Documentation was updated
Affected artifact templates in packages/cli were updated
Affected example projects in packages/example-* were updated

shimks · 2018-03-15T16:48:01Z

Have these files been manually verified after prettier's been run? If they haven't, should they be verified?

virkt25 · 2018-03-15T16:57:53Z

Verified in what sense? Prettier changes - to * and applied the 80 character limit to all md files. Don't think it did anything else. I did skim through the files and didn't spot anything else.

shimks

Just one comment

shimks · 2018-03-15T17:00:07Z

README.md

-Raymond Feng|Miroslav Bajtos|Ritchie Martori|Kevin Delisle
-:-:|:-:|:-:|:-:
-[<img src="https://avatars0.githubusercontent.com/u/540892?v=3&s=60">](http://github.com/raymondfeng)|[<img src="https://avatars2.githubusercontent.com/u/1140553?v=3&s=60">](http://github.com/bajtos)|[<img src="https://avatars2.githubusercontent.com/u/462228?v=3&s=60">](http://github.com/ritch)|[<img src="https://avatars3.githubusercontent.com/u/2053534?v=3&s=60">](http://github.com/kjdelisle)
+|                                             Raymond Feng                                              |                                          Miroslav Bajtos                                          |                                         Ritchie Martori                                         |                                            Kevin Delisle                                             |


The readability is pretty horrendous here. Should we configure prettier so that it doesn't run on tables?

I'm not sure how to disable it for just tables in md files. Any ideas?

I don't think it's possible to do it just for tables in prettier. Curious to hear others' opinion on this.

Perhaps prettier/prettier#3211 might help.

Great find! I've updated the tables (with the exception of @shimks 's image since I'm not sure how to set the height attribute in reference style and GitHub's s=60 isn't working for his Avatar).

raymondfeng · 2018-03-15T17:56:02Z

.prettierrc

+      "files": "**/*.md",
+      "options": {
+        "parser": "markdown",
+        "proseWrap": "always"


Should it be preserve? See https://prettier.io/docs/en/options.html#prose-wrap

preserve didn't enforce lines to be max 80 characters in width (which is what I'm trying to achieve with this PR). Hence went with always.

raymondfeng · 2018-03-15T17:56:59Z

.github/PULL_REQUEST_TEMPLATE.md

- [ ] Related API Documentation was updated
- [ ] Affected artifact templates in `packages/cli` were updated
- [ ] Affected example projects in `packages/example-*` were updated
+* [ ] `npm test` passes on your machine


Hmm, why * is better than -? Is there a configuration option to override?

I've looked into this a while ago, and prettier doesn't have configurable list styling, and it doesn't look to be in scope for a while. prettier/prettier#3025

No config option other than to extend the markdown parser used by prettier and change that there. * and - are treated the same by GFM (GitHub Flavoured Markdown). I think the markdown parser just uses * as it's style.

In VSCode, you can continue using - and saving the file will format it to use * now.

Forcing to use * is fairly annoying :-(

Yeah, I also like - more than *. However, I am happy to give up this preference in exchange for automated linting & formatting of Markdown files.

Perhaps we can contribute a feature to prettier to make the preference of * vs. - configurable? They already allow configuration of the quote symbol used in JavaScript files (' or "), I think that should serve as a good precedent?

b-admike · 2018-03-15T18:07:57Z

.prettierignore

 *.json
-*.md
+CHANGELOG.md
+.sandbox


can we add packages/*/CHANGELOG.md here?

I think CHANGELOG.md already covers nesting files with the same name.

raymondfeng · 2018-03-15T20:11:28Z

README.md

+[superkhau]: https://avatars1.githubusercontent.com/u/1617364?v=3&s=60
+[loay]: https://avatars3.githubusercontent.com/u/1986928?v=3&s=60
+[crandmck]: https://avatars2.githubusercontent.com/u/2925364?v=3&s=60
+[virkt25]: https://avatars1.githubusercontent.com/u/3311536?v=3&s=60


bajtos · 2018-03-16T14:29:40Z

+1 for applying Prettier to all markdown files

@virkt25 could you please check that the VS Code extension we use to apply Prettier formatting is correctly configured to reformat Markdown files on save? I remember turning automatic reformatting off - see https://github.com/strongloop/loopback-next/blob/20a41ac7081a8cead0011447b5a8e5794a320ded/.vscode/settings.json#L7-L9

bajtos · 2018-03-16T14:31:35Z

I remember turning automatic reformatting off

Ah, that was done for EJS templates, not markdown - see c1d7a52

virkt25 · 2018-03-16T14:40:10Z

Yep, the VSCode extension works great in re-formatting the files. It automatically does word wrapping to 80 characters, adjusts tables based on content, changes - to *, etc.

connected to #1099

virkt25 · 2018-03-20T21:10:53Z

@strongloop/lb-next-dev

Do we want to proceed with this PR or should it be abandoned / closed?

PROS:

We get linting on markdown files (80 character word wrap is applied for us automatically ... which should make reviews easier for future.

CONS:

Lists switch from - to *. For newly generated lists that use -, prettier will covert them to *. For existing lists, we must use * because otherwise Prettier will treat the new addition of - as a new list as per the CommonMark spec / guidelines. This is a feature of Prettier. It doesn't allow for config of default list bullets. See issue: Prettier mangles lists with mixed list syntaxes prettier/prettier#3871
By using this plugin https://github.com/neilsustc/vscode-markdown, we can have a newline automatically give us a bullet (same pointer as list). This could be a solution for this issue_

raymondfeng · 2018-03-20T21:23:37Z

I still feel being forced to use * instead of - is pretty annoying. Please note that we have to use Shift + 8 to type * while - is one key stroke.

virkt25 · 2018-03-26T14:31:47Z

Closing this as we want prettier to support - for lists instead of * before adding this to linting.

virkt25 added this to the March 2018 milestone Mar 15, 2018

virkt25 self-assigned this Mar 15, 2018

virkt25 requested review from b-admike, dhmlau, jannyHou, raymondfeng and shimks March 15, 2018 16:33

virkt25 requested a review from bajtos as a code owner March 15, 2018 16:33

virkt25 added the needs-priority label Mar 15, 2018

virkt25 force-pushed the style/md branch from 38e78c1 to 590e6ef Compare March 15, 2018 16:35

virkt25 force-pushed the style/md branch from 459407b to effbc20 Compare March 15, 2018 16:56

shimks reviewed Mar 15, 2018

View reviewed changes

raymondfeng reviewed Mar 15, 2018

View reviewed changes

b-admike reviewed Mar 15, 2018

View reviewed changes

raymondfeng reviewed Mar 15, 2018

View reviewed changes

b-admike approved these changes Mar 16, 2018

View reviewed changes

virkt25 force-pushed the style/md branch 2 times, most recently from 62360d7 to f7f713b Compare March 20, 2018 20:44

chore: enable prettier for markdown files

472b901

connected to #1099

virkt25 force-pushed the style/md branch from f7f713b to 472b901 Compare March 20, 2018 20:45

virkt25 added 2 commits March 20, 2018 16:51

style: apply prettier rules to all markdown files

5d52db4

chore: use reference style for images in readme

de3dd38

raymondfeng mentioned this pull request Mar 21, 2018

style: format markdown files for consistency #1164

Merged

6 tasks

virkt25 closed this Mar 26, 2018

This was referenced Apr 3, 2018

Should use the most popular unordered list style prettier/prettier#3025

Closed

Markdown Unordered List Item Symbol prettier/prettier#4251

Closed

bajtos deleted the style/md branch June 5, 2018 07:51

shimks mentioned this pull request Jun 11, 2018

chore: add prettier settings for md files #1412

Merged

7 tasks

Conversation

virkt25 commented Mar 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

shimks commented Mar 15, 2018

Uh oh!

virkt25 commented Mar 15, 2018

Uh oh!

shimks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virkt25 Mar 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bajtos commented Mar 16, 2018

Uh oh!

bajtos commented Mar 16, 2018

Uh oh!

virkt25 commented Mar 16, 2018

Uh oh!

virkt25 commented Mar 20, 2018

Uh oh!

raymondfeng commented Mar 20, 2018

Uh oh!

virkt25 commented Mar 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

virkt25 commented Mar 15, 2018 •

edited

Loading

virkt25 Mar 15, 2018 •

edited

Loading