Skip to content
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

CI: Fix editorconfig #7478

Merged
merged 3 commits into from
Nov 26, 2023
Merged

CI: Fix editorconfig #7478

merged 3 commits into from
Nov 26, 2023

Conversation

szepeviktor
Copy link
Contributor

👀

@@ -1,6 +1,7 @@
root = true

[*]
charset = utf-8
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Character set was missing

.editorconfig Outdated
Comment on lines 13 to 14
[{*.yml, *.yaml}]
[*.{yml,yaml,md,json}]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was invalid and incomplete

.editorconfig Outdated
@@ -10,5 +11,5 @@ trim_trailing_whitespace = true
[tests/Fixtures/**]
trim_trailing_whitespace = false

[{*.yml, *.yaml}]
[*.{yml,yaml,md,json}]
Copy link
Member

Choose a reason for hiding this comment

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

Not all existing files following this rule. Mind to apply it first ?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about the same, but in the end I just thought that it will be applied anyway when files are modified. But maybe applying it all at once is better - for me not a strict requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🏃🏻‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 🍏

Copy link
Member

Choose a reason for hiding this comment

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

This file must be left as-is because it's auto-generated and it would lead to constant diffs (previous format would be restored after running the command) or would require to apply formatting in IDE before committing. We need to revert this change and ignore it somehow. I see that people suggest generated_code = true but I don't see this in official specs. Found the issue where ... = unset can be used for un-setting requirements. Or maybe we just can revert it and do not apply it again - nobody edits baseline manually anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
If baseline is never opened in an EC-compatible IDE we should not care about it.

BTW I use EC for validation, not only as suggestions. https://github.com/greut/eclint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note

linguist-generated attribute (in .gitattributes) could be used to flag generated files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

                git ls-files --cached \
                    | git check-attr --stdin --all \
                    | sed -n -e 's#^\(.\+\): linguist-generated: set$#":!:\1"#p' \
                    | xargs -- git grep --null --files-with-matches -I -e '.' -- \
                    | xargs --null -n 1 -- any command ...

@keradus keradus changed the title chore: Fix editorconfig CI: Fix editorconfig Nov 26, 2023
@Wirone Wirone merged commit e1672a4 into PHP-CS-Fixer:master Nov 26, 2023
25 checks passed
@Wirone
Copy link
Member

Wirone commented Nov 26, 2023

Thank you @szepeviktor 🍻

@keradus
Copy link
Member

keradus commented Nov 26, 2023

some files are still misconfigured.
eg prlint.json using 2 spaces, composer.json using 4

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Nov 26, 2023

some files are still misconfigured.
eg prlint.json using 2 spaces, composer.json using 4

I see!
It is hard to spot 4 space indentation when having 2 as the desired value.
2×2=4

@keradus
Copy link
Member

keradus commented Nov 26, 2023

It's even worse!
2+2=4, also!

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.

None yet

3 participants