Dobiasd / FunctionalPlus

Functional Programming Library for C++. Write concise and readable C++ code.
http://www.editgym.com/fplus-api-search/
Boost Software License 1.0
2.07k stars 168 forks source link

Auto-format code #291

Closed Dobiasd closed 6 months ago

Dobiasd commented 7 months ago

So far, no auto-formatting was applied (at least by me), which resulted in a somewhat inconsistent code format.

Dobiasd commented 7 months ago

Currently, I don't have a strong opinion on the exact formatter-setting details to use. But I guess it should be something simple, so for this MR I used the default in VSCode (ms-vscode.cpptools).

@offa @pthom @tom91136 Do you see anything fundamentally wrong with this or with auto-formatting in general for this project?

pthom commented 7 months ago

Hello Tobias,

On my side, I do not see anything wrong with using auto-formatting. It does break the history of files (i.e. git blame will likely stop at such a commit), but in the case of fplus I think this is a minor issue.

If you are interested in this, you might want to use editorconfig. It requires a plugin with Visual Studio Code. I cannot say I have a lot of experience with it, but it would be a way to communicate the formatting choices, and let some editors take it into account.

I do think that 80 columns is a bit to low for a default (100 looks more sensible to me).

Beware, it seems that the auto-formatting went a bit too far, replacing

               view::ints(0)
                | view::take(15000000)
                | view::transform(times_3)
                | view::remove_if(is_odd_int)
                | view::transform(as_string_length)
                , 0);

by

                view::ints(0) | view::take(15000000) | view::transform(times_3) | view::remove_if(is_odd_int) | view::transform(as_string_length), 0);

(which is 135 wide)

Merry christmas!

tom91136 commented 7 months ago

I usually use https://clang.llvm.org/docs/ClangFormat.html which is supported by VSCode and CLion, and possibly most other IDEs. That way we can keep it all the same (as opposed to being tied to the default of the specific version of the IDE plugin) by writing a .clang-format, here's mine for example:

---
AllowShortIfStatementsOnASingleLine: Always
AllowShortCaseLabelsOnASingleLine: true
AllowShortFunctionsOnASingleLine: All
AlignEscapedNewlines: Left
IndentCaseLabels: true
ColumnLimit: 140
CompactNamespaces: true
FixNamespaceComments: true
IndentPPDirectives: BeforeHash
...

I generated mine using https://clang-format-configurator.site/

And Merry Christmas! Sorry about opening the two PRs basically on Xmas/boxing day and thanks for reviewing those.

Dobiasd commented 7 months ago

@pthom Oh, thanks for the remark. I did not notice this ugly format change. And, yes, git blame is a good point. :+1: But in this project, I guess I should usually be the person to blame initially anyway. :grin: Happy Holidays to you too! :heart:

Dobiasd commented 7 months ago

@tom91136 ClangFormat looks good. :+1: I'll play a bit with it and update this PR. And don't worry about PRs on xmas. If I would not want to respond on such days, I feel it's on me to manage my emails/notifications/todos/whatever according to my availability, same as receiving text messages during the night (of the recipient's timezone). :wink: And I enjoyed reviewing/discussing the PRs. :nerd_face:

Dobiasd commented 6 months ago

For simplicity, I tried the predefined styles of clang-format (LLVM, GNU, Google, Chromium, Microsoft, Mozilla, WebKit) and visually liked WebKit the most. It also does not destroy the code example mentioned by pthom. :tada:

offa commented 6 months ago

It might be a good idea to verify formatting as part of the CI build once merged. I can work on this if helping hands are needed. :+1:

Dobiasd commented 6 months ago

@offa That sounds cool! In case it's not too complex, and you already have an idea of how to implement it, I'd be happy if you could help. :heart:

Later, I'd then maybe use whatever you implement here for frugally-deep too. :slightly_smiling_face: