Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: format repository with tabs#740

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
ignatiusmb wants to merge9 commits intosveltejs:master
base:master
Choose a base branch
Loading
fromignatiusmb:format-with-tabs

Conversation

ignatiusmb
Copy link
Member

@ignatiusmbignatiusmb commentedJan 6, 2021
edited
Loading

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

Originally posted by@dummdidumm in#735 (comment)


Thefirst commit formats all the files using the existing config (for an easier look at the relevant diffs only) and thethird commit formats it again afteruseTabs 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 usesmarkdownlint 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)
  • changetabWidth to 2 as well
  • addmarkdownlint config file to format markdowns

@dummdidumm
Copy link
Member

dummdidumm commentedJan 6, 2021
edited
Loading

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
MemberAuthor

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 onlypackage.jsons (or all.json) to not use tabs and 2 spaces indentation. Do we want to use tabs for all exceptpackage.json or enforce the same style aspackage.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]
imageimageimage

@ignatiusmb
Copy link
MemberAuthor

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 spacestabWidth only applies topackage.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.

@ignatiusmbignatiusmb marked this pull request as ready for reviewApril 16, 2021 14:44
@ignatiusmb
Copy link
MemberAuthor

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 freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dummdidummdummdidummdummdidumm left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ignatiusmb@dummdidumm

[8]ページ先頭

©2009-2025 Movatter.jp