Skip to content
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

Prettier pre-work: formatting tweaks #832

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

benjie
Copy link
Member

@benjie benjie commented Apr 2, 2021

In #727 I have been working on prettier support for formatting the GraphQL spec. The PR is formed of three commits: one manually applying changes to the markdown to minimize the resulting diff, one adding the prettier tooling, and finally one that runs prettier against the codebase.

When the following three PRs to spec-md are merged and released, the diff of the generated HTML on that final commit will be very small (mostly whitespace within tables due to the new table formatting):

(Edit: all these have been merged and Lee's advanced spec-md further 🙌 this means that final diff is now EMPTY!)

This PR is the first commit from #727: the manual formatting changes that were required. My hope is that we can merge this now and then reviewing the prettier formatted PR will be significantly less work (because the output HTML will be identical).

The changes in this PR are broadly:

  • Change to standard markdown titles (support added in spec-md)
  • Escape special markdown characters that may confuse prettier
  • Formatting code blocks more like how prettier will format them
  • Marking code blocks that should not be formatted as raw (support added in spec-md)

Marking this as draft because I want to validate my changes before others spend time looking at it.

@benjie benjie marked this pull request as ready for review April 5, 2021 09:51
@benjie
Copy link
Member Author

benjie commented Apr 5, 2021

I've reviewed these changes and I'm happy they are all deliberate.

@benjie
Copy link
Member Author

benjie commented Apr 5, 2021

I've rebased this on main so that it incorporates the latest spec-md. I've re-reviewed the HTML diff with the latest spec-md and only expected changes are seen 👍

@benjie
Copy link
Member Author

benjie commented Apr 5, 2021

The latest changes to spec-md mean that the prettier PR (#727) now renders identical HTML to this PR - excellent work @leebyron! 🙌

@@ -1494,7 +1500,7 @@ For example, the following query will not pass validation because `@skip` has
been used twice for the same field:

```graphql counter-example
query ($foo: Boolean = true, $bar: Boolean = false) {
query($foo: Boolean = true, $bar: Boolean = false) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a prettier bug. Just filed prettier/prettier#10655

@leebyron leebyron merged commit b47598f into graphql:main Apr 5, 2021
@benjie benjie deleted the pre-prettier-tweaks branch April 5, 2021 22:26
@leebyron leebyron added the ✏️ Editorial PR is non-normative or does not influence implementation label Apr 6, 2021
@leebyron leebyron added this to the May2021 milestone Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ Editorial PR is non-normative or does not influence implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants