Skip to content

chore: Add EditorConfig for IDE whitespace #1032

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

Merged
merged 2 commits into from
Nov 6, 2020
Merged

Conversation

nschonni
Copy link
Contributor

@nschonni nschonni commented Nov 3, 2020

Why:

Allows IDEs and GitHub web editor to detect the whitespace settings when editing files https://editorconfig.org/

What's being changed:

Add a basic .editorconfig file, but don't run any normalizaton

Check off the following:

@janiceilene
Copy link
Contributor

Thanks @nschonni! I'll get this triaged for review ✨

@janiceilene janiceilene added the engineering Will involve Docs Engineering label Nov 4, 2020
@chiedo
Copy link
Contributor

chiedo commented Nov 5, 2020

@nschonni just confirming, this .editorconfig file as you've proposed it, would make the following editors automatically conform to our whitespacing preferences that we check against with our linting?

Screen Shot 2020-11-05 at 11 02 54 AM
?

@nschonni
Copy link
Contributor Author

nschonni commented Nov 5, 2020

Yes, the whitespace settings are picked up, so when you hit the Tab key, it inserts the indicated number of spaces, and will add the EOF marker if it is missing.
For VS Code, you can added the plug-in as a recommended extension https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig, by adding a extensions.json in the .vscode folder with the following:

{
	"recommendations": ["EditorConfig.EditorConfig"]
}

Didn't add that in this PR to keep the discussion focused, but I can add that in if you want.

It looks like standard that you use for the JS linting doesn't have native support for it. Standard is an oppionated wrapper around ESLint. ESLint itself recommends the whitespace rules be delegated to Prettier, which does respect EditorConfig 😆

I had taken a quick look at what flipping to plain ESLint + Prettier would look like, but it reformatted so much I didn't want to submit it out of the blue

Copy link
Contributor

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

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

This looks great to me, thanks for the addition @nschonni! If I was going to write a .editorconfig file, this is exactly what I'd write 📝 I'll leave it to @chiedo or @janiceilene to actually merge this once they're ready, but 👍 from an eng perspective. ✨

@chiedo chiedo closed this Nov 6, 2020
@chiedo chiedo reopened this Nov 6, 2020
@chiedo chiedo merged commit 4a5fddd into github:main Nov 6, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2020

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours.

@mnoori1201

This comment was marked as spam.

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

Successfully merging this pull request may close these issues.