Skip to content

Commit

Permalink
[clang-format] Update config.
Browse files Browse the repository at this point in the history
  • Loading branch information
@egorpugin
egorpugin committed Mar 12, 2021
1 parent 0e9deb6 commit afa476b
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions .clang-format
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
---
BasedOnStyle: Google
BasedOnStyle: Google
# Only merge empty functions.
AllowShortFunctionsOnASingleLine: Empty
# Do not allow short if statements.
AllowShortIfStatementsOnASingleLine: false
# Enforce always the same pointer alignment.
DerivePointerAlignment: false
IndentPPDirectives: AfterHash

PointerAlignment: Right
IncludeBlocks: Preserve
FixNamespaceComments: true
ColumnLimit: 100
IndentWidth: 2
#IndentAccessModifiers: false # not accepted atm
AccessModifierOffset: -2 # set to minus IndentWidth (-IndentWidth)
SpacesBeforeTrailingComments: 1
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
BreakConstructorInitializers: BeforeComma
#ConstructorInitializerAllOnOneLineOrOnePerLine: false

9 comments on commit afa476b

@stweil
Copy link
Member

@stweil stweil commented on afa476b Mar 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The old style was the result of a community discussion. I'd prefer if such changes were discussed, too, before applying them.

@egorpugin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What to change?

@stweil
Copy link
Member

@stweil stweil commented on afa476b Mar 13, 2021

Choose a reason for hiding this comment

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

@amitdo, @Shreeshrii, @zdenop, what do you think?

I'd stick as close as possible to the previous settings and the Google style.

PointerAlignment was left by default. I prefer right, but we agreed on left, so I'd remove that setting and use the default.
FixNamespaceComments: true can be removed because that's the default.
ColumnLimit is 80 by default. I'd remove that setting and use the default.
IndentWidth: 2 can be removed because that's the default.
AccessModifierOffset is -1 by default. I'd remove that setting and use the default.
SpacesBeforeTrailingComments is 2 by default. I'd remove that setting and use the default.
AllowShortIfStatementsOnASingleLine is a duplicate which can be removed.
AllowShortLoopsOnASingleLine is true by default. Changing that to false is fine for me.
BreakConstructorInitializers is BeforeColon by default. I currently don't see whether that makes a difference.

@egorpugin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have a link to previous discussion?

Notes:

  • Some things could be reconsidered now.
  • It's fine to keep default options explicit.
  • (not a format note) By default google does not use references and sometimes the intentions are not clear - can be passed parameter nullptr or can not.

Style:

  1. pointers - right is used in linux, llvm. I also prefer right.
  2. access modifier offset - ok, could be indent/2.
  3. spaces before comments - one is fine, two is unnecessary. Why to press twice? On namespaces } // namespace tesseract double space is even uglier.
  4. Loops on single line are not very readable.
  5. Remove duplicate - yes.
  6. Also it will be better to sort whole config.

@egorpugin
Copy link
Contributor Author

@egorpugin egorpugin commented on afa476b Mar 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Also 'access modifier offset' is not very visible when is not on the same level with class. It becomes hidden in this case

class X {
 public:
  void something();
  lots
  of
  more
  stuff
 private:
  some
  other
  one
  by
  one
  lines

vs.

class X {
public:
  void something();
  lots
  of
  more
  stuff
private:
  some
  other
  one
  by
  one
  lines

@stweil
Copy link
Member

@stweil stweil commented on afa476b Mar 13, 2021

Choose a reason for hiding this comment

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

Do you have a link to previous discussion?

See the issues #854 and #1598 which contain discussion and additional links.

@stweil
Copy link
Member

@stweil stweil commented on afa476b Mar 13, 2021

Choose a reason for hiding this comment

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

The Google C++ Style Guide is here: https://google.github.io/styleguide/cppguide.html.

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on afa476b Mar 13, 2021

Choose a reason for hiding this comment

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

I think we should let Stefan choose his preferences because he is the main contributor for several years now.

@GerHobbelt
Copy link
Contributor

Choose a reason for hiding this comment

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

Fully agree with @amitdo.

You worry about community and democratic principles here, but those assume a relatively level playing field, where everyone contributes something.
That is not suitable / applicable here.

The question should be: is tesseract code legible, does the formatting negatively impact readability? (How easy is RTFC?)

My position is: it's good enough as it is. If you feel you need to tweak a few bits, fine. Go ahead.
Only danger is changing it too much. What is "too much"?

I've been writing C code (and related languages) for 30+ years now. While I developed a strong opinion on code formatting and was pretty adament about it 20 years ago, there's something I had to accept over time. (I "learned" it, but it was a process going from having to tolerate to accepting, rather.) What I learned is this:

  • if you work on a codebase that irritates you, you MAY change it. This has consequences:

    • you are now responsible for keeping the code formatted that way, no matter who is adding what.
    • without tools, that is a significant burden: if your formatting tool can't do it, you're one unlucky bastard.
    • uncrustify or equivalent powerful tools may be found and used, but now you have a problem when you have an incoming stream of changes you want/need to track. Nowadays that's when you are a git fork or get input from other "groups". Not applicable to y'all as you're tesseract mainline, but worth to keep in mind nevertheless: anything like a Code Summer has the same effect (affect? Sorry, English...)
    • I've done it for years. If you're not super well tooled up for git merge conflicts resolution (clashes due to git going ape over format changes close to functional changes), you're SOL. The corrollary of this is: if your toolset can't do it, then your preference about that bit will loose because it will cost continually. Hence: learn to accept. "Live with it."

  • I've spent my entire life in business environments with lots of code. While there's nothing democratic about a business (it's much closer to a military hierarchy; when the leadership is okay with having a managed democracy in their own backyard, that's great but not a, ah, "human right". What was given can be taken away.) the codebases are perhaps best categorized as meritocratic (of a kind): quality and survival depend entirely on who is willing to work on it.
    Long observation and logical reasoning has led me to this: it doesn't matter how much money ("encouragement") you throw at it, if there are no people internally motivated (for whatever reason) to work on the pile, it will die. May take years, but it's a zombie until you terminate it anyway.

    This implies that the most important people to keep a product alive are not the salesmen (valuable as they are) but the maintainers who "stick around and plod on".

    Open Source has a different encouragement strategy (no salaries due, no stock options, etc.) but the codebases have their own life and it's the same: when no-one is intrinsically willing to be around and stay around and keep it alive, it's maybe not dead, but "frozen" like a mammoth: comparable cost / effort required to get it back to "alive" status again. (In this analogy it can be as costly as a mammoth or as easy as reviving a yeast kernel, but I digress.)

Me, I'm more seagull like: my attention wanders. Can't help it, that's my nature. Like that old little robot from an old movie: "need input! need input!"
I can be around, move away, return later, etc. That's not a good fit for a maintainer. Fortunately for tesseract, the product, there are a few who do have maintainer mindset. You know who you are. You have my respect!

And when in doubt, do a git log analysis and measure the timespan of one's commits, NOT necessarily the volume (in LOC or other "KPI").

You are, by default, the bosses and you MUST decide what makes you yourselves happy and motivated. The only real factor you have to contend with is the history: the code that already exists. As such, that is an important contributor and should be taken into account. But that is not a "living" contributor who has or needs motivation, so it may decide your departure point, yet you are the ones who must decide how you want to see it tomorrow.

My point being?!

I contributed the equivalent of a single semicolon. As a mere mote part if your community, I'm trying to tell you this:

I need tesseract alive and surviving long term or I can scrap my own projects / goals right now!

While I appreciate you considering community opinion, I and anyone else who isn't a de facto maintainer, can only give you one intelligent response: thank you for considering us, but: Change what personally truely IRKS you, anything that makes you happy and is fully automatable with ease.
Anyone else with a brain will follow you. It may be bothering some of us for a bit, but eff that: the smart choice there is learning to accept it. I will follow, and I need you to pick what you need to want to get back to this tomorrow. And any day after that.

Amen.

After this sunday sermon, I would like to close with a few opinionated notes:

  • please DO NOT move to a google style spec because you may feel that might be "better". Google now is like a 1950's Coke bottle: it's a mandatory fashion statement for anyone who wants to belong with the perceived cool kids. The truely hard part is deciding for yourselves (Stefan, Shree, Egon? Sorry if I missed one, mea culpa) how you would like the code to look and read. That's bloody tough and when you stick to your guns, that's a good potential indicator for leadership in my book. Whatever you do, you have my vote.

  • do not change a lot at once. Been there, done that, got the (faded) t-shirt. Be smarter than I ever was and change a few bits, see how it works for you, WAIT, then tweak that one other bit that's bothering. Maybe do a release, then a new release with formatting changes only so outside world can observe and follow along. I don't know.

  • only hard systems requirement: any formatting rule must be fully enforcible by fully automated formatter tooling. If configuring and setting that up takes (much) effort, ditch the rule and try to learn living with what the automaton gives you. While I detest having to adapt myself to match the machine (rather than vice versa) that's sometimes the cheapest and "best" path long term.

  • on the left/right thing: muscle memory makes me type 'void *ptr' even if I don't want to, my rational brain ran into 'void* ptr' long ago, railed against it and then grudgingly agreed that 'void*' keeps the entire type together very nicely, making it preferable from a structural point of view ("clean code") while even today there's still the emo part of my brain who finds '*ptr' far more important as it somehow helps my reasoning about the code: I need that little extra hint of spicy-ness to remind my inner computer simulator there's a pointer about and 'void' can stick it up its jumper as far as that brainCPU is concerned. Alas.

    Together with curly braces and indent (switch and case at same level please so end curlies don't "jump/gap" at a switch in the flow) that is a religious war that will never go away as different individuals perceive code differently (hm, grokking code is perhaps the better wording here...)

Hence my plea to minimally change, only tweaking the truly irksome bits and ignoring-while-aware the siren song of any other parochy. (Unless you're into collecting Coke bottles, of course. The key is: enjoy what you are writing and reading. /Our/ task is to accept the gift as is. Your track record is sane, so that should be doable for us all.)

That's my second semicolon-sized contribution here. My hope is this helps giving you guidance, whatever you choose.

Thank you for your time; I will shut up now.

Please sign in to comment.