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

Format astro files in examples #3862

Merged
merged 6 commits into from
Jul 8, 2022
Merged

Format astro files in examples #3862

merged 6 commits into from
Jul 8, 2022

Conversation

MarcusOtter
Copy link
Member

@MarcusOtter MarcusOtter commented Jul 8, 2022

Changes

This PR runs the prettier-plugin-astro on the example projects to close #3855.

I did this by first installing the plugin in the repo, then running npx prettier --write ./examples/**/*.astro, then reverting the setup commit (since we do not want to add it yet, as discussed in the issue).

Testing

I have not had time to test all the examples, would very much appreciate if someone could help me with that! It should only be a formatting change but maybe we'll find some edgecase where it breaks behavior? EDIT: I guess the CI should take care of testing that anyways.

Docs

No docs change needed

@changeset-bot
Copy link

changeset-bot bot commented Jul 8, 2022

⚠️ No Changeset found

Latest commit: ab3c302

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: example Related to an example package (scope) label Jul 8, 2022
Comment on lines 12 to 18
<nav></nav>
<div class='wrapper mt4 mb4'>
<h1>Page Not Found</h1>
<p>Not found</p>
</div>
<footer />
<footer></footer>
</body>
Copy link
Member

Choose a reason for hiding this comment

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

This is unrelated to the formatting changes, but isn't this file supposed to use the imported components? 😅

Copy link
Member Author

@MarcusOtter MarcusOtter Jul 8, 2022

Choose a reason for hiding this comment

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

Good catch! Maybe it should be another PR?

@Princesseuh
Copy link
Member

Princesseuh commented Jul 8, 2022

This looks good! Just three things:

What I suggest we do is:

  • Rerun it on the examples with singleQuotes set to false, just so we get proper quotes at least
  • Close slots manually
  • Move the style tag manually

That way we can at least get this PR merged without needing to fix all the issues (that we'll definitely fix in the future, of course!). If you don't have time to do it, don't hesitate to tell me and I'll take over this, very happy to help on this!

@MarcusOtter MarcusOtter requested a review from Princesseuh July 8, 2022 18:37
Copy link
Member

@Princesseuh Princesseuh left a comment

Choose a reason for hiding this comment

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

Looks good to me, there was a few changes I wasn't sure of inside the portfolio example but testing locally, there seems to be no issues! Once this is merged, another PR adding the commit to this file will be needed

@MarcusOtter
Copy link
Member Author

MarcusOtter commented Jul 8, 2022

Thanks again! I was going to go above and beyond to use ' for inside code fences, I even made this beautiful regex for it to replace them all at once, but VSCode didn't support variable length lookbehinds 😢

If anyone is interested in doing it manually, this regex works in VSCode to select all codefences with double quotes in them: ---(.|\n)*"+(.|\n)*---, and this is the regex that would only select the double quotes, but wasn't supported in VSCode: (?<=---(.|\n)*)"+(?=(.|\n)*---)

Regextester VSCode
bild bild

(and yes, I did spend more time trying to automate it than it would've taken to do it manually)

@MarcusOtter MarcusOtter merged commit 59e8c71 into withastro:main Jul 8, 2022
@MarcusOtter MarcusOtter deleted the prettier-plugin-astro-files branch July 8, 2022 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: example Related to an example package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Very inconsistent formatting in default example projects
2 participants