Skip to content

New Feature: Add .editorconfig Support#259

Merged
TomasVotruba merged 7 commits into
mainfrom
kesills-editorconfig
Jan 31, 2025
Merged

New Feature: Add .editorconfig Support#259
TomasVotruba merged 7 commits into
mainfrom
kesills-editorconfig

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

@TomasVotruba TomasVotruba commented Jan 31, 2025

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 #217
Comment thread src/Configuration/ECSConfigBuilder.php Outdated
@TomasVotruba TomasVotruba changed the title kesills editorconfig New Feature: Add .editorconfig Support Jan 31, 2025
@TomasVotruba
Copy link
Copy Markdown
Member Author

@samsonasik Could I ask you for review here? I'll resolve and finish it at once then

@TomasVotruba
Copy link
Copy Markdown
Member Author

Awesome :)

@TomasVotruba TomasVotruba merged commit 73135c6 into main Jan 31, 2025
@TomasVotruba TomasVotruba deleted the kesills-editorconfig branch January 31, 2025 13:05
@TomasVotruba
Copy link
Copy Markdown
Member Author

Thank you @Kenneth-Sills 🚀

class EditorConfig
{
public function __construct(
public readonly ?IndentStyle $indentStyle,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it seems due to enum usage

enum IndentStyle: string
{
    case Space = 'space';
    case Tab = 'tab';
}

I think we can just use class and constant.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I created new PR to fix it

by use class and constant instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

New Feature: Add .editorconfig Support

3 participants