Fixes #150

Mar 13 '18 22:03 jakelang

Don't ever do this.

Mar 14 '18 08:03 chfast

@chfast ? was working on fixing it

Mar 14 '18 11:03 jakelang

You can fix it, but reformatting all the code will never be accepted. You should propose .clang-format config first. I recommend the one from ethminer.

Mar 14 '18 11:03 chfast

Sorry I closed it prematurely.

Mar 14 '18 11:03 chfast

ah ok. will roll back then. This format file is from cpp-ethereum

Mar 14 '18 11:03 jakelang

About fixing it: clang-format reorders the includes (if asked). This can be the reason for build errors.

Mar 16 '18 10:03 chfast

Apparently this was not the issue, as I had tried earlier to simply switch around the only two includes.

Mar 16 '18 22:03 jakelang

This has not been touched in a long time. Do we still want to add a format file? cc @chfast @axic

May 11 '18 00:05 jakelang

I'm really for it. It can be different style, but it would make my life easier if I could reformat my changes with a single click.

May 11 '18 08:05 chfast

What style would you prefer?

May 11 '18 12:05 jakelang

Me? This one: https://github.com/ethereum-mining/ethminer/blob/master/.clang-format.

May 11 '18 12:05 chfast

Alright, I am using the ethminer clang-format file. I have no preference for the style outside of plain C. @axic any thoughts about this?

May 11 '18 22:05 jakelang

I do not really like formatting changes like this:

-bool hasWasmPreamble(vector<uint8_t> const& _input) {
-  return
-    _input.size() >= 8 &&
-    _input[0] == 0 &&
-    _input[1] == 'a' &&
-    _input[2] == 's' &&
-    _input[3] == 'm' &&
-    _input[4] == 1 &&
-    _input[5] == 0 &&
-    _input[6] == 0 &&
-    _input[7] == 0;
+namespace
+{
+bool hasWasmPreamble(vector<uint8_t> const& _input)
+{
+    return _input.size() >= 8 && _input[0] == 0 && _input[1] == 'a' && _input[2] == 's' &&
+           _input[3] == 'm' && _input[4] == 1 && _input[5] == 0 && _input[6] == 0 && _input[7] == 0;
 }

The second one is way harder to follow. Is there a way to keep the former one?

Jul 29 '18 12:07 axic

I am all for having this merged, but couldn't so far find a config fulfilling all my requirements :)

Jul 29 '18 12:07 axic

Seems like the webkit style is the closest: https://webkit.org/code-style-guidelines/

Need to check these out:

BasedOnStyle (string) The style used for all options not specifically set in the configuration.

This option is supported only in the clang-format configuration (both within -style='{...}' and the .clang-format file).

Possible values:

LLVM A style complying with the LLVM coding standards Google A style complying with Google’s C++ style guide Chromium A style complying with Chromium’s style guide Mozilla A style complying with Mozilla’s style guide WebKit A style complying with WebKit’s style guide

Jul 29 '18 12:07 axic

If you want to experiment with it,

make sure you have the latest version of clang-format (6),
dump the config of the style you thinks is the closest:

clang-format-6.0 -style=WebKit -dump-config > .clang-format

. You will be able to see all available options.

Jul 30 '18 10:07 chfast

Yeah I am reading the documentation and have a pretty good version now.

Jul 30 '18 10:07 axic

I do not really like formatting changes like this:

-bool hasWasmPreamble(vector<uint8_t> const& _input) {

return

_input.size() >= 8 &&

_input[0] == 0 &&

_input[1] == 'a' &&

_input[2] == 's' &&

_input[3] == 'm' &&

_input[4] == 1 &&

_input[5] == 0 &&

_input[6] == 0 &&

_input[7] == 0; +namespace +{ +bool hasWasmPreamble(vector<uint8_t> const& _input) +{
return _input.size() >= 8 && _input[0] == 0 && _input[1] == 'a' && _input[2] == 's' &&
      _input[3] == 'm' && _input[4] == 1 && _input[5] == 0 && _input[6] == 0 && _input[7] == 0;
}

I don't think there is any option to change how the expressions are packed. If you want hand-crafted expressions, wrap them with // clang-format off.

Jul 30 '18 10:07 chfast

Can I use it for new code?

Aug 14 '18 17:08 chfast

Rebased.

Dec 11 '18 01:12 jakelang

Codecov Report

Merging #151 into master will increase coverage by 1.12%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   50.89%   52.02%   +1.12%     
==========================================
  Files           8        8              
  Lines        1342     1311      -31     
  Branches      130      129       -1     
==========================================
- Hits          683      682       -1     
+ Misses        632      602      -30     
  Partials       27       27

Dec 11 '18 01:12 codecov-io

@chfast please note I'll adjust the settings, just wanted to push CI first.

Feb 13 '19 13:02 axic

I've noticed you touched this PR, so I dumped the style I'm currently using for Hera. I don't care which one is used in the end, but having one is big improvement for my daily workflow.

Feb 13 '19 13:02 chfast

What's the progress?

Feb 26 '19 15:02 chfast

@chfast pushed some updates which are a bit closer to the current and Solidity's style. What do you think?

I still can't get over how ugly it formats wavm.cpp with those EEI definitions.

Apr 30 '19 17:04 axic

I will take a look next week. I've notices clang-format has problems with formatting arrays of lambdas.

You don't want to format CMake files...

Apr 30 '19 18:04 chfast

You don't want to format CMake files...

Yeah the final version of this PR won't do that.

Apr 30 '19 18:04 axic

Should copy the rules from https://github.com/wasmx/fizzy.

Jun 09 '20 14:06 axic

Add .clang-format file

Codecov Report