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

chore: format repository with tabs #740

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ignatiusmb
Copy link
Member

@ignatiusmb ignatiusmb commented Jan 6, 2021

This repo uses spaces for intendation (maybe we should align that with the main repo some day ...)

Originally posted by @dummdidumm in #735 (comment)


The first commit formats all the files using the existing config (for an easier look at the relevant diffs only) and the third commit formats it again after useTabs is changed to true (we can be sure that the diffs are only on the spaces). There's a couple of file extensions intentionally not included like

  • package.json files, which is designed to uses spaces (main repo too)
  • *.md files, which the Svelte code blocks now gets formatted too and usually uses markdownlint to format it as well
  • *.json files, feels weird to format with tabs and 4 spaces

Proposed changes

  • always use arrow parens (applied on the first commit, not sure why)
  • change tabWidth to 2 as well
  • add markdownlint config file to format markdowns

@dummdidumm
Copy link
Member

dummdidumm commented Jan 6, 2021

Thanks for having a crack at this!

When you say "intentionally not included", what do you mean by that? Prettier did format them but you did not include the change?

@ignatiusmb
Copy link
Member Author

Yes, I formatted the whole repo and excluded those files from the commit. I was hoping to get some feedback first before I add any additional config settings here.

For *.json files, they're formatted as expected. But, package.json could be considered an exception where it usually uses 2 spaces for indentation. I could add a config to override only package.jsons (or all .json) to not use tabs and 2 spaces indentation. Do we want to use tabs for all except package.json or enforce the same style as package.json for .json files?

For *.md files, they're still formatted... but, it's result is inconsistent. It won't format where there's a mix of space and tabs[1], code blocks are messed up[2], and for some reason it decides to format list items to have 3 spaces between the first character[3]. I decided to leave it for the time being, but I could fix it manually since they're relatively few in number (a total of 10 files), perhaps in a different PR.

[1] [2] [3]
image image image

@ignatiusmb
Copy link
Member Author

Sorry this took a while to get back to, but I think I finally got it!

Looking at the main repo for reference, it seems that, .json files are indeed formatted with tabs and 4 spaces too. I've also strike the last 2 points as 2 spaces tabWidth only applies to package.json. As for the markdown files, I still think it's better to use markdownlint to help lint them if we're going to fully utilize .md for documentations, but that can wait in another PR.

@ignatiusmb ignatiusmb marked this pull request as ready for review April 16, 2021 14:44
@ignatiusmb
Copy link
Member Author

In some rare cases, prettier will delete some lines in code blocks with Svelte language identifier. Somehow, explicitly setting the sort order helps calm it down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants