Added .clang-format + script for consistent coding style #10
Conversation
.clang-format
Outdated
| AlignTrailingComments: true | ||
| AllowAllParametersOfDeclarationOnNextLine: true | ||
| AllowShortFunctionsOnASingleLine: Empty | ||
| AllowShortIfStatementsOnASingleLine: Never |
There was a problem hiding this comment.
Let's allow short if statements on a single line -- it adds unnecessary size to our files otherwise and can make things a bit harder to read.
| binaryenLeft, | ||
| binaryenRight | ||
| ); | ||
| return BinaryenStringConcat(module, binaryenLeft, binaryenRight); |
There was a problem hiding this comment.
Is there any way to make this reformatting? Often times when I write each function argument on its own line its to improve readability. In the case above it would make sense for it to be reformatted to this. However here I would have wanted to keep it on separate lines.
There was a problem hiding this comment.
there is an option for this : https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket but i didn't get it to work even with the "Align" value. Though i managed to solve the mentioned problem by reducing the maxColumnLimit so that it forces it to destruct it's paramatres into separate lines.(only for the long example). Maybe needs more looking into. I'm still learning clang-format so there could be an option that i'm missing
| static void generateSource(shared_ptr<SourceNode> node, BinaryenModuleRef &module); | ||
| static BinaryenModuleRef generateWasmFromAST(shared_ptr<ASTNode> ast); | ||
| static BinaryenExpressionRef generate(shared_ptr<ASTNode> node, BinaryenModuleRef &module); | ||
| static BinaryenExpressionRef generateBinaryOperation(shared_ptr<BinaryOperationNode> node, |
There was a problem hiding this comment.
Is there a way to get this to format to:
static BinaryenExpressionRef generateBinaryOperation(
shared_ptr<BinaryOperationNode> node,
BinaryenModuleRef &module
);instead?
There was a problem hiding this comment.
It looks like this is still happening -- any idea why?
| string toJSON(); | ||
|
|
||
| static string tokenTypeToString(Token::Types token) { | ||
| static map<Token::Types, string> tokensMap = {{Token::Types::STRING, "STRING"}, |
There was a problem hiding this comment.
We lose a ton of whitespace this way -- can we change it to keep the old formatting in this case?
Mehdibenhadjkhelifa
force-pushed
the
feature/clang-format
branch
2 times, most recently
from
2f42bb1
to
587954a
Compare
|
requests fulfilled. Only 1 request that has some look into(about each argument being in a separate line) you can check out my comment on your request for more info about that. Hope this helps! Cheers ! |
|
@Mehdibenhadjkhelifa okay yeah that makes sense. Let's just move the column limit back to 120 for now and I'll see if I can figure out how to fix that last case |
|
@Mehdibenhadjkhelifa I think you'll have to reset the changes to all the files except the clang config stuff and then re-run the formatter to fix all the cases where the single-line if statements were changed to multiline. But once you do that and the max col width change I'll go ahead and merge this in |
Mehdibenhadjkhelifa
force-pushed
the
feature/clang-format
branch
from
587954a
to
a2c3740
Compare
|
Changed as request.For the if's i thought you just wanted the ifs only to be single line. not all ifs else-ifs and elses. So i adjust the option to do that accordingly and reset the maxcolumnlength to be 120( this unmade a lot of what i changed since some parametres i didn't know how to change or they don't have it for the specific request ) even though changing it to 100 made better changes overall but i changed as per requested. |
This is to make the coding style across the project consistent when multiple people work on it. I have already used the script on the project to format source files. If you dislike some styles you can make changes to the .clang-format file as you see fit .Hope this helps!