Skip to content

Add an .editorconfig file #212

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 1 commit into from
Jun 2, 2021

Conversation

oliverklee
Copy link
Collaborator

This helps contributors get the basic formatting options in their IDEs
automatically set right. (The setting for PHP files complies to the
PSR-12 standard.)

This change does not autoformat any files with these settings, though.

@oliverklee oliverklee force-pushed the feature/editorconfig branch from b792149 to 407bb64 Compare April 21, 2021 17:38
@oliverklee oliverklee marked this pull request as draft April 21, 2021 18:07
@oliverklee
Copy link
Collaborator Author

Hold on, I think Markdown should have lines no longer than 80 characters. I'll do some more research on best practices there.

@oliverklee oliverklee force-pushed the feature/editorconfig branch 2 times, most recently from 2fa83c5 to aa2051e Compare May 5, 2021 10:54
@oliverklee oliverklee force-pushed the feature/editorconfig branch from aa2051e to 467fa3a Compare May 30, 2021 13:01
@oliverklee oliverklee marked this pull request as ready for review May 30, 2021 13:01
@oliverklee oliverklee force-pushed the feature/editorconfig branch from 467fa3a to 762f1d1 Compare May 30, 2021 13:01
.editorconfig Outdated
trim_trailing_whitespace = false

[*.css]
indent_style = tab
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not quite sure about this. Currently, the CSS test files are indented with tabs, and so this setting would keep this tabs. Or we could (in a separate PR) convert the CSS files to also use spaces for consistency. I'd prefer the second option and would be okay with both. :-)

@westonruter @sabberworm What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And should we allow (semi-)unlimited line lengths for the CSS files (as some files have compressed CSS)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've looked through some more CSS files: Their indentation is are a colorful mixture of tabs, 2 spaces and 4 spaces. :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, adding an editorconfig doesn’t mean the files need to be reformatted, it’s just a hint to the IDE on how such a file should be formatted.
I think it’s OK for fixtures to differ in indentation since the parser needs to be able to handle all styles found in the wild. Some fixtures are just real-life cases of files that the parser couldn’t handle correctly, which is why they’re committed as-is.
Personally I vastly prefer tab indentation over spaces but I prefer consistency even more so I don’t think we need to special-case *.css here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I've remove the CSS special case now and repushed.

This helps contributors get the basic formatting options in their IDEs
automatically set right. (The setting for PHP files complies to the
PSR-12 standard.)

This change does not autoformat any files with these settings, though.
@oliverklee oliverklee force-pushed the feature/editorconfig branch from 762f1d1 to 1e0e7e6 Compare June 1, 2021 21:00
@sabberworm sabberworm merged commit 1a874ce into MyIntervals:master Jun 2, 2021
@oliverklee oliverklee deleted the feature/editorconfig branch June 2, 2021 08:05
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.

3 participants