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

Auto-Formatting #2248

Open
thorstenhater opened this issue Jan 16, 2024 · 5 comments
Open

Auto-Formatting #2248

thorstenhater opened this issue Jan 16, 2024 · 5 comments
Assignees
Labels
AEP Arbor Enhancement Proposal code quality enhancement

Comments

@thorstenhater
Copy link
Contributor

Hi all,

let's make our lives a bit easier and use an auto formatter. So, decision time.

Zero: Do we?

If we go for it, we'll have to stick with it, make some compromises on style, and accept
a potentially humongous PR with all the initial change. The payoff is the peace of mind
of never having to care again.

First: How?

Based on the lay of land the only real options are clang-format and uncrustify. The former
is tricky to use and realistically must be pinned to one version.

Second: What?

This is the hard part. Once we have decided on a tool, we'll go through all the options and pick
those we like best, collectively.

@halfflat
Copy link
Contributor

I've always been against it as you know, primarily because it adversely affects readability of mathematical expressions. But I also acknowledge that my opinion should carry little weight given that I'm contributing very little.

That said, I will also continue to claim that it is a very low burden to contribute code in the house style, whether that aim be accomplished by hand or with the assistance of local tools.

@thorstenhater
Copy link
Contributor Author

Sure, your concerns are valid and input is always welcome. Have you taken a look at uncrustify?
I know of your earlier spat with clang-format, but here might a light at the end of the tunnel:

#
# Spacing options
#

# Add or remove space around non-assignment symbolic operators ('+', '/', '%',
# '<<', and so forth).
sp_arith                        = ignore   # ignore/add/remove/force/not_defined

# Add or remove space around arithmetic operators '+' and '-'.
#
# Overrides sp_arith.
sp_arith_additive               = ignore   # ignore/add/remove/force/not_defined

I noted the willingness to compromise as a requirement above. The trade-off
is that we'll all get 80%+ of what we want of a style, but we also never have to
nit anyone else in review for slight deviation of the perceived correct formatting.

@halfflat
Copy link
Contributor

Ahah! I will take a closer look at uncrustify — my concerns may well now be moot.

@thorstenhater
Copy link
Contributor Author

It seems a saner alternative to clang-format.

@Helveg
Copy link
Collaborator

Helveg commented Jan 30, 2024

For Python I would recommend black (with a longer line length) and isort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AEP Arbor Enhancement Proposal code quality enhancement
Projects
None yet
Development

No branches or pull requests

6 participants