-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
First pass with clang-format #198
base: master
Are you sure you want to change the base?
Conversation
} | ||
|
||
//============================================================================== | ||
void AppHarness::watch (const juce::File& f) | ||
void AppHarness::watch(const juce::File& f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we trying to stick to the juce style guide? If so I think non-empty parens would have a preceding space (i.e. SpaceBeforeParens: NonEmptyParentheses
instead of SpaceBeforeParens: ControlStatements
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh. Hopefully we don't need to fully comply .... I hate that leading space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, such a can of worms this business. I'm in the not caring so long as a tool is doing it for me camp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, I'll admit, I'm also not a fan of that leading space. Generally though, I too am in the "i don't care let's just standardize it" camp.
I think generally if we can reach a small consensus of "yea looks good enough for me" then we should go for it
return (this->timeoutsManager.get()->*method)(_args.arguments[0]); | ||
} | ||
})); | ||
registerNativeProperty(name, juce::var::NativeFunction([this, name, method](const juce::var::NativeFunctionArgs& _args) -> juce::var { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of the way this indents the inline lambda here... is there a way to nudge this back as it was?
duk_destroy_heap | ||
); | ||
duk_create_heap(nullptr, nullptr, nullptr, nullptr, detail::fatalErrorHandler), | ||
duk_destroy_heap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not a huge fan of pulling this trailing comma up. But my clang-format knowledge is weak here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah man: https://reviews.llvm.org/D33029
Big change, but all this really does is use the clang-format file that @JoshMarler mentioned in #24 with juce-like defaults and run it against
blueprint/core/*
Let me know what you think!