Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Use clang-format for auto-format (replaces libastyle) #11543

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

Open
cmaglie wants to merge 12 commits into arduino:master
base: master
Choose a base branch
Loading
from cmaglie:clang-format

Conversation

Copy link
Member

@cmaglie cmaglie commented May 19, 2021 •
edited
Loading

As per the subject, clang-format is used instead of AStyle.

The clang-format formatting style cannot match the current AStyle formatting style because clang-format has much stricter formatting rules than AStyle and is not possible to provide a configuration that is 100% matching between the two.

clang-format is a mature and maintained tool and it's used in the Arduino IDE 2.0 too.

The "Arduino" clang-format configuration is bundled within the IDE and it's used by default. It may be customized by the user by:

  • adding a .clang-format configuration file in the sketch folder (so the new style is applied only to the sketch)
    or
  • adding a .clang-format configuration file in the data directory, the same directory as the preferences.txt file (in this case the style is applied globally)

per1234 and bensuperpc reacted with thumbs up emoji
cmaglie added 6 commits May 19, 2021 12:17 
Still missing installation of clang-format binaries via build.xml
Being a plugin Tool is not mandatory and, in this case, it just makes
things more complicated without any evident benefit.
@arduino arduino deleted a comment from ArduinoBot May 21, 2021
@arduino arduino deleted a comment from ArduinoBot May 21, 2021
@arduino arduino deleted a comment from ArduinoBot May 21, 2021
@cmaglie cmaglie marked this pull request as ready for review May 21, 2021 13:40
Copy link

ladyada commented May 21, 2021

yay!
silvanocerza, umbynos, and rsora reacted with heart emoji

Comment on lines +994 to +995
//menu.add(createToolMenuItem("processing.app.tools.CreateFont"));
//menu.add(createToolMenuItem("processing.app.tools.ColorSelector"));
Copy link

@silvanocerza silvanocerza May 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can't we just remove these comments?

Copy link

Good one, it also taught me a lesson when it comes to one-liner loops 😬

Copy link

@ladyada
any chance your team can check if this format aligns with the one used in your clang-format step in the PR CI?

Copy link

ladyada commented May 28, 2021

hey00 we use clang-format-8 and believe me we have learned they are not all the same so you really have to pick one and stick to it. that said, if y'all want to pick one clang format version i'm happy to change our CI to match :)

https://github.com/adafruit/ci-arduino/blob/master/actions_install.sh#L8

per1234 reacted with thumbs up emoji ubidefeo reacted with rocket emoji

editor.statusNotice(tr("Auto Format finished."));
} catch (IOException | InterruptedException e) {
editor.statusError("Auto format error: " + e.getMessage());
e.printStackTrace();
Copy link

@abimarank abimarank Dec 23, 2021

Choose a reason for hiding this comment

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

Care to removee e.printStackTrace();?

Copy link
Collaborator

per1234 commented Aug 8, 2022 •
edited
Loading

we have learned they are not all the same so you really have to pick one and stick to it. that said, if y'all want to pick one clang format version i'm happy to change our CI to match :)

Hi @ladyada. Thanks so much for this advice. It caused me to give more attention to the need to carefully evaluate the impact of updates to the version of ClangFormat on the output of the Arduino IDE's "Auto Format" feature.

If the recent development history is any indication, it is likely Arduino will continue to update to new versions of ClangFormat periodically. We have created a system for efficiently and reliably validating these updates:

https://github.com/arduino/tooling-project-assets/tree/main/other/clang-format-configuration

This system allows for updates to the ClangFormat version in use while ensuring consistent results for the Arduino IDE users.

The system is focused on the maintenance of Arduino's custom ClangFormat configuration file, whereas from a quick look at a few random repos it appears Adafruit is using the default configuration, so you may have slightly different considerations. However, clang-format --dump-config can be used to output the effective configuration of the tool, so really not so different after all.

Arduino IDE 2.x is currently using ClangFormat 14.0.0. Even though there was an astonishing number of changes to the configuration options since the 11.0.1 version used at the time Arduino's custom configuration was developed, backwards compatibility with the old configuration was retained and the diff between the output of the two versions was restricted to code using a few newer C++ language features.

Copy link

ladyada commented Aug 10, 2022

sounds good - yes one day we will probably update to -11, but of course it could require a big sweep to assure that we can go through and fix all the libraries to pass. mostly clang-format really helps keep PRs reasonable - we would get a lot of PRs with tons of whitespace changes and clangformatting at least keeps the formatting consistant so we can review PRs without noise :)

@per1234 per1234 added Component: IDE The Arduino IDE feature request A request to make an enhancement (not a bug fix) labels May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@abimarank abimarank abimarank requested changes

@silvanocerza silvanocerza silvanocerza approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Component: IDE The Arduino IDE feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /