Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add clang-format for a common source code format #1365

Merged
merged 2 commits into from
Mar 16, 2019
Merged

Add clang-format for a common source code format #1365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7fc5414
Add clang-format
maicki Mar 5, 2019
c94c75e
Update format to be as close as our current style
maicki Mar 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
BasedOnStyle: Google
maicki marked this conversation as resolved.
Show resolved Hide resolved

## Indentation
ColumnLimit: 120
IndentWidth: 2

## Line Break
BreakBeforeBraces: Custom
BraceWrapping:
AfterClass: true
AfterControlStatement: false
AfterEnum: false
AfterFunction: true
AfterNamespace: true
AfterObjCDeclaration: false
AfterStruct: false
AfterUnion: true
BeforeCatch: false
BeforeElse: false
IndentBraces: false
SplitEmptyFunction: false

## ObjC
ObjCBinPackProtocolList: Always
ObjCSpaceAfterProperty: false

## Comments
AlignTrailingComments: true

## Arguments / Parameters

# If false, a function call’s or function definition’s arguments will either
# all be on the same line or will have one line each.
BinPackArguments: false

# If false, a function call’s or function definition’s parameters will either
# all be on the same line or will have one line each.
BinPackParameters: false

# Allow putting all parameters of a function declaration onto the next line even
# if BinPackParameters is false.
# For example this should be ok:
# someFunction(foo,
# bar,
# baz);
AllowAllParametersOfDeclarationOnNextLine: true

## Single Line
AllowShortBlocksOnASingleLine: false
AllowShortFunctionsOnASingleLine: Empty
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false

42 changes: 42 additions & 0 deletions format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/bash
#
# Script uses clang-format to format all of the source files in a
# default format.
#

# The absolute path of the directory containing this script.
DIR="$( cd "$( dirname "$0" )" && pwd)"
# Define top level project directory
PROJECT_DIR="${DIR}"

METHOD="GIT"
if [ -n "$1" ]; then
METHOD=$1
fi

# Paths to format
MONITOR=()
MONITOR+=( "${PROJECT_DIR}/Source" )

if [ "$METHOD" == "ALL" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Are we planning to run this script for "all" anytime soon? If not then maybe don't include this mode for now for simpler script?

Copy link
Member

Choose a reason for hiding this comment

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

I guess, the right question would be: do we want to run this script for "all" anytime soon? Having a huge diff that touches so many files will affect the usability of git blame/history. I think a much more lenient approach would be to have it run on new PRs for a while, and then run "all" months/a year from now. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Also, can you share a new diff with the latest format? a08cc7b is outdated right?

Copy link
Contributor Author

@maicki maicki Mar 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The plan would be to run it all at once together with the introduction of the script.

Correct it will affect to the blame / history, but having it one at for all I think would make more sense as gradually and having problems with blame / history one file after another.

a08cc7b is the latest, I updated the diff with the latest changes morning.

Copy link
Member

Choose a reason for hiding this comment

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

ok. In that case, what do you think about keeping the open brackets as is?

Copy link
Member

Choose a reason for hiding this comment

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

I think git blame -w would sidestep most of the pain, but that's only when running it from the command-line, for i.e. github, sourcetree, IntelliJ, where I think most people actually use it, there's no way to inject that flag into the command :(

# Format all Source files
for FILE in $(find ${MONITOR[*]} -type f \( -iname "*.mm" -o -iname "*.m" -o -iname "*.h" \))
do
clang-format -style=file -i "$FILE" &
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
clang-format -style=file -i "$FILE" &
clang-format -style=file -i "$FILE"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The & is there on purpose. It runs the command in the background and does not wait for every file.

Copy link
Member

Choose a reason for hiding this comment

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

ahh, got it!

done
elif [ "$METHOD" == "GIT" ]; then
# Gather all added or modified files from git and format them
CHANGED_FILES=$(git ls-files --other --modified --exclude-standard | grep ".*[\.m|\.mm|\.h|\.hpp]$")
for CHANGED_FILE in $CHANGED_FILES
do
clang-format -style=file -i "${PROJECT_DIR}/$CHANGED_FILE" &
done
else
echo "Usage: $0 [GIT|ALL]";
echo "GIT: Only format added or modified files"
echo "ALL: Format all files"
fi


# Wait until all clang-format invoctations are done
wait