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

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented May 19, 2021

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)

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
… support

The places checked for a valid config are:

- the sketch folder
- the global data folder (behind preferences.txt)
- the Arduino IDE app dir

The default config is copied in the Arduino IDE app dir
@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
@ladyada
Copy link

ladyada commented May 21, 2021

yay!

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

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?

@ubidefeo
Copy link

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

@ubidefeo
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?

@ladyada
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

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

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();?

@per1234
Copy link
Collaborator

per1234 commented Aug 8, 2022

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.

@ladyada
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
Labels
Component: IDE The Arduino IDE feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants