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

migrate from astyle to clang-format, add tests/ #8455

Closed
wants to merge 15 commits into from

Conversation

AllmanStyler
Copy link

@AllmanStyler AllmanStyler commented Jan 19, 2022

edit3

@mcspr
Copy link
Collaborator

mcspr commented Jan 19, 2022

a lot of changes :>
where does braces rule come from? seems to be the most common one here

Comment on lines 21 to 22
typedef enum
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

<- I meant this

Comment on lines +122 to +124
do
{
} while (do_menu());
Copy link
Collaborator

Choose a reason for hiding this comment

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

...and this

Comment on lines +22 to +23
auto t = [](const char* h, const char* n)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

...and this, which is correct from the ruleset standpoint, but looks really weird :/

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it is sometimes weird.
If we had another tool than astyle we'd have less oddities.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Macros also seem to break b/c of the braces detachment
I'd expect clang-format to work pretty well, with a more verbose ruleset though compared to the astyle

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also see https://sourceforge.net/p/astyle/bugs/548/
Plus, for examples (where astyle conf Arduino IDE formatter link is 404 btw) IDE 2.0+ is expected migrate to clang tool as well arduino/Arduino#11543

Copy link
Collaborator

@d-a-v d-a-v Jan 19, 2022

Choose a reason for hiding this comment

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

Is this better ?
I'll try to find the arduino-example configuration when I get more time (edit: done, see OP)

(edit: any change to the configuration file is welcome and can be tested in this PR)

@d-a-v d-a-v changed the title apply Allman style to tests/ migrate from astyle to clang-format, add tests/ Jan 20, 2022
@mcspr mcspr mentioned this pull request Jan 25, 2022
@d-a-v
Copy link
Collaborator

d-a-v commented Mar 3, 2022

Closing in favor of #8464

@d-a-v d-a-v closed this Mar 3, 2022
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