Skip to content

add format script #11

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
104 changes: 104 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
---
# SPDX-FileCopyrightText: 2019 Christoph Cullmann <[email protected]>
# SPDX-FileCopyrightText: 2019 Gernot Gebhard <[email protected]>
#
# SPDX-License-Identifier: MIT

# This file got automatically created by ECM, do not edit
# See https://clang.llvm.org/docs/ClangFormatStyleOptions.html for the config options
# and https://community.kde.org/Policies/Frameworks_Coding_Style#Clang-format_automatic_code_formatting
# for clang-format tips & tricks
---
Language: JavaScript
DisableFormat: true
---

# Style for C++
Language: Cpp

# base is WebKit coding style: https://webkit.org/code-style-guidelines/
# below are only things set that diverge from this style!
BasedOnStyle: WebKit

# enforce C++11 (e.g. for std::vector<std::vector<lala>>
Standard: Cpp11

# 4 spaces indent
TabWidth: 4
IndentWidth: 4
UseTab: Always

SpacesInLineCommentPrefix:
Minimum: 1
Maximum: 1

# 2 * 80 wide lines
ColumnLimit: 160

# sort includes inside line separated groups
SortIncludes: true

# break before braces on function, namespace and class definitions.
BreakBeforeBraces: Custom
BraceWrapping:
AfterClass: false
AfterCaseLabel: false

# CrlInstruction *a;
DerivePointerAlignment: false
PointerAlignment: Left
ReferenceAlignment: Left

# only clang format 14
# RemoveBracesLLVM: true

#QualifierAlignment: Left

# horizontally aligns arguments after an open bracket.
AlignAfterOpenBracket: Align

# don't move all parameters to new line
AllowAllParametersOfDeclarationOnNextLine: false

# no single line functions
AllowShortFunctionsOnASingleLine: None

# always break before you encounter multi line strings
AlwaysBreakBeforeMultilineStrings: true

# don't move arguments to own lines if they are not all on the same
BinPackArguments: false

# don't move parameters to own lines if they are not all on the same
BinPackParameters: false

# In case we have an if statement with multiple lines the operator should be at the beginning of the line
# but we do not want to break assignments
BreakBeforeBinaryOperators: NonAssignment

# format C++11 braced lists like function calls
Cpp11BracedListStyle: true

# do not put a space before C++11 braced lists
SpaceBeforeCpp11BracedList: false

# remove empty lines
KeepEmptyLinesAtTheStartOfBlocks: false

# no namespace indentation to keep indent level low
NamespaceIndentation: None

# we use template< without space.
SpaceAfterTemplateKeyword: false

# Always break after template declaration
AlwaysBreakTemplateDeclarations: true

# macros for which the opening brace stays attached.
ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH, forever, Q_FOREVER, QBENCHMARK, QBENCHMARK_ONCE , wl_resource_for_each, wl_resource_for_each_safe ]

# keep lambda formatting multi-line if not empty
AllowShortLambdasOnASingleLine: Empty

# We do not want clang-format to put all arguments on a new line
AllowAllArgumentsOnNextLine: false
9 changes: 9 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,13 @@ jobs:
working-directory: ${{github.workspace}}/build
# Didn't configure to use ctest. Opted for a custom target to run instead
run: cmake --build ${{github.workspace}}/build --target test -- -j

format:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Test format
run: |
./scripts/cl-fmt.sh
git diff --exit-code

Comment on lines +30 to 38
Copy link
Owner

Choose a reason for hiding this comment

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

What if we used the clang format action? This would allow us to drop the script and pull the action.

Suggested change
format:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Test format
run: |
./scripts/cl-fmt.sh
git diff --exit-code
format-check:
name: Format checks
runs-on: ubuntu-latest
strategy:
matrix:
path:
- 'src'
- 'test'
- 'include'
steps:
- uses: actions/checkout@v3
- name: Test format
uses: jidicula/clang-format-action@v4
with:
clang-format-version: '15'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes this is possible, the benefit of the script is that it can be run locally, so if you would like to format your code, just execute the script. But if you like we can also use the action.
I would first prefer to fix the codebase and then adding this.

Copy link
Owner

Choose a reason for hiding this comment

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

That makes sense. We can keep the script then. I was seeing your action file was using it and running the diff. Wasn't actually formatting then committing and a color diff would have been nice. I stumbled across this and figured it made more sense to do this and run the format on it one time locally like you said.

If you want I can run it and merge or you can do that within this one. Whatever works best for the work you are doing let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to first merge the other two branches, because otherwise a merge gets really a mess. So this is on hold for now

Copy link
Owner

Choose a reason for hiding this comment

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

okay that is what I figured you wanted. Adding a TODO after that.

  • Format code with clang format here

41 changes: 41 additions & 0 deletions scripts/cl-fmt.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#!/bin/bash

# Variable that will hold the name of the clang-format command
FMT=""

FOLDERS=("src" "test")

# Some distros just call it clang-format. Others (e.g. Ubuntu) are insistent
# that the version number be part of the command. We prefer clang-format if
# that's present, otherwise we check clang-format-13
for clangfmt in clang-format{,-13}; do
if which "$clangfmt" &>/dev/null; then
FMT="$clangfmt"
break
fi
done

# Check if we found a working clang-format
if [ -z "$FMT" ]; then
echo "failed to find clang-format. Please install clang-format version 13 or above"
exit 1
fi

function format() {
# ignore getRSS.h file
for f in $(find $@ -name '*.h' ! -iname 'getRSS.h' -or -name '*.m' -or -name '*.mm' -or -name '*.c' -or -name '*.cpp'); do
echo "format ${f}";
${FMT} -i ${f};
done

echo "~~~ $@ Done ~~~";
}

# Check all of the arguments first to make sure they're all directories
for dir in ${FOLDERS[@]}; do
if [ ! -d "${dir}" ]; then
echo "${dir} is not a directory";
else
format ${dir};
fi
done