Skip to content

New Feature: Add .editorconfig Support #234

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

Conversation

Kenneth-Sills
Copy link
Contributor

The approach I've gone for puts .editorconfig settings above those settings in the ecs.php config itself, to prioritize convenience for those people using pre-built sets. As many setttings as I could easily implement have been supported, including one proposed (but not standardized) setting I like to use.

As mentioned in the related ticket, I've used a similar approach as Prettier does, only pulling configuration settings from [*] and [*.php] at the root-level .editorconfig (no recursive traversal to root), since we don't have immediate access to a high quality EditorConfig library and don't want to maintain anything that complicated.

The changes made here should also be backwards-compatible for a minor version release; opt-in for support is required for the new feature, and all other changes make the checker more permissive. But please do check my work, especially in regard to the changed conflicting / duplicate checker entries.

This commit includes tests, documentation, and dogfooding.

Tests only currently cover the parsing of .editorconfig, but do not test how all permutations affect configuration. Doing so would require advanced alias mocks of the EditorConfigFactory and it's load method, which I wasn't able to get working in an initial attempt. That, or a refactor of ECSConfigBuilder to allow injecting the factory, but I didn't want to mess with any of the exposed public API.

Closes #217


Thank you for your time! 😄

Found these while adding EditorConfig support.

This is NOT a breaking change, since this will only make the tool MORE
permissive.
The approach I've gone for puts `.editorconfig` settings above those settings in
the `ecs.php` config itself, to prioritize convenience for those people using
pre-built sets. As many setttings as I could easily implement have been
supported, including one proposed (but not standardized) setting I like to
use.

This commit includes tests, documentation, and dogfooding.

Tests only currently cover the parsing of `.editorconfig`, but do not test
how all permutations affect configuration. Doing so would require advanced
alias mocks of the `EditorConfigFactory` and it's `load` method, which I wasn't
able to get working in an initial attempt. That, or a refactor of
`ECSConfigBuilder` to allow injecting the factory, but I didn't want to mess
with  any of the exposed public API.

Closes easy-coding-standard#217
@TomasVotruba
Copy link
Contributor

Thanks for this feature. Seems like a good addition 👍

I made a rebased PR here:
#259

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.

New Feature: Add .editorconfig Support
2 participants