From fd1b7ba52901179c19f5997b650518993d919f52 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Fri, 1 Apr 2016 20:48:11 -0700 Subject: support making fish code match the style guide This changes implements two new make targets: `style` and `style-all`. These make it easy to ensure that a change conforms to the project style guides for C++ and fish code. Fixes #571 --- .clang-format | 8 ++++ CONTRIBUTING.md | 109 +++++++++++++++++++++---------------------------- Makefile.in | 10 ++++- build_tools/lint.fish | 21 ++++------ build_tools/style.fish | 90 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 161 insertions(+), 77 deletions(-) create mode 100644 .clang-format mode change 100755 => 100644 build_tools/lint.fish create mode 100755 build_tools/style.fish diff --git a/.clang-format b/.clang-format new file mode 100644 index 00000000..d154da60 --- /dev/null +++ b/.clang-format @@ -0,0 +1,8 @@ +# Use the Google style with these modifications: +# +# 1) lines can be up to 100 chars long rather than 80, and +# 2) use a four space indent rather than two spaces. +# +BasedOnStyle: Google +ColumnLimit: 100 +IndentWidth: 4 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6321cdc7..af2ad690 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,27 +1,31 @@ # Guidelines For Developers +This document provides guidelines for making changes to the fish-shell project. This includes rules for how to format the code, naming conventions, etc. It also includes recommended best practices such as creating a Travis-CI account so you can verify your changes pass all the tests before making a pull-request. + +See the bottom of this document for help on installing the linting and style reformatting tools discussed in the following sections. + ## Lint Free Code -Automated analysis tools like cppcheck and oclint can help identify bugs. They also help ensure the code has a consistent style and avoids patterns that tend to confuse people. +Automated analysis tools like cppcheck and oclint can point out potential bugs. They also help ensure the code has a consistent style and that it avoids patterns that tend to confuse people. Ultimately we want lint free code. However, at the moment a lot of cleanup is required to reach that goal. For now simply try to avoid introducing new lint. To make linting the code easy there are two make targets: `lint` and `lint-all`. The latter does just what the name implies. The former will lint any modified but not committed `*.cpp` files. If there is no uncommitted work it will lint the files in the most recent commit. -To install the lint checkers on Mac OS X using HomeBrew: +## Ensuring Your Changes Conform to the Style Guides + +The following sections discuss the specific rules for the style that should be used when writing fish code. To ensure your changes conform to the style rules you simply need to run ``` -brew tap oclint/formulae -brew install oclint -brew install cppcheck +make style ``` -To install the lint checkers on Linux distros that use Apt: +before commiting your change. If you've already committed your changes that's okay since it will then check the files in the most recent commit. This can be useful after you've merged someone elses change and want to check that it's style is acceptable. + +If you want to check the style of the entire code base run ``` -sudo apt-get install clang -sudo apt-get install oclint -sudo apt-get install cppcheck +make style-all ``` ## Fish Script Style Guide @@ -34,75 +38,54 @@ The first word of global variable names should generally be `fish` for public va ## C++ Style Guide -1. The `clang-format` command is authoritative with respect to indentation, whitespace around operators, etc. **Note**: this rule should be ignored at this time. A subsequent commit will add the necessary config file and make targets. After the happens the code will be cleaned up and this rule will become mandatory. - -1. All names in code should be `small_snake_case`. No Hungarian notation is used. Classes and structs names should be followed by `_t`. - -1. fish uses the Allman/BSD style of indentation. - -1. Indent with spaces, not tabs. - -1. Use 4 spaces per indent. - -1. Opening curly bracket is on the following line: - - // ✔: - struct name - { - // code - }; +1. The [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) forms the basis of the fish C++ style guide. There are two major deviations for the fish project. First, a four, rather than two, space indent. Second, line lengths up to 100, rather than 80, characters. - void func() - { - // code - } +1. The `clang-format` command is authoritative with respect to indentation, whitespace around operators, etc. **Note**: this rule should be ignored at this time. After the code is cleaned up this rule will become mandatory. - if (...) - { - // code - } +1. All names in code should be `small_snake_case`. No Hungarian notation is used. Classes and structs names should be followed by `_t`. - // ✗: - void func() { - // code - } +1. Always attach braces to the surrounding context. -1. Put space after `if`, `while` and `for` before conditions. +1. Indent with spaces, not tabs and use four spaces per indent. - // ✔: - if () {} +## Installing the Required Tools - // ✗: - if() {} +### Installing the Linting Tools -1. Put spaces before and after operators excluding increment and decrement; +To install the lint checkers on Mac OS X using HomeBrew: - // ✔: - int a = 1 + 2 * 3; - a++; +``` +brew tap oclint/formulae +brew install oclint +brew install cppcheck +``` - // ✗: - int a=1+2*3; - a ++; +To install the lint checkers on Linux distros that use Apt: -1. Never put spaces between function name and parameters list. +``` +sudo apt-get install clang +sudo apt-get install oclint +sudo apt-get install cppcheck +``` - // ✔: - func(args); +### Installing the Reformatting Tools - // ✗: - func (args); +To install the reformatting tool on Mac OS X using HomeBrew: -1. Never put spaces after `(` and before `)`. +``` +brew install clang-format +``` -1. Always put space after comma and semicolon. +To install the reformatting tool on Linux distros that use Apt: - // ✔: - func(arg1, arg2); +``` +apt-cache search clang-format +``` - for (int i = 0; i < LENGTH; i++) {} +That will list the versions available. Pick the newest one available (3.6 for Ubuntu 14.04 as I write this) and install it: - // ✗: - func(arg1,arg2); +``` +sudo apt-get install clang-format-3.6 +sudo ln -s /usr/bin/clang-format-3.6 /usr/bin/clang-format - for (int i = 0;i&1 + cppcheck -q --verbose --std=posix --std=c11 --language=c++ --template "[{file}:{line}]: {severity} ({id}): {message}" --suppress=missingIncludeSystem --inline-suppr --enable=$cppchecks $cppcheck_args $c_files 2>& 1 end if type -q oclint @@ -82,20 +78,19 @@ if set -q c_files[1] oclint-xcodebuild xcodebuild.log > /dev/null end if test $all = yes - oclint-json-compilation-database -e '/pcre2-10.20/' \ - -- -enable-global-analysis 2>&1 + oclint-json-compilation-database -e '/pcre2-10.20/' -- -enable-global-analysis 2>& 1 else set i_files for f in $c_files set i_files $i_files -i $f end echo oclint-json-compilation-database -e '/pcre2-10.20/' $i_files - oclint-json-compilation-database -e '/pcre2-10.20/' $i_files 2>&1 + oclint-json-compilation-database -e '/pcre2-10.20/' $i_files 2>& 1 end else # Presumably we're on Linux or other platform not requiring special # handling for oclint to work. - oclint $c_files -- $argv 2>&1 + oclint $c_files -- $argv 2>& 1 end end else diff --git a/build_tools/style.fish b/build_tools/style.fish new file mode 100755 index 00000000..76cf920d --- /dev/null +++ b/build_tools/style.fish @@ -0,0 +1,90 @@ +#!/usr/bin/env fish +# +# This is meant to be run by "make style" or "make style-all". It is not meant to +# be run directly from a shell prompt although it can be. +# +# This runs C++ files and fish scripts (*.fish) through their respective code +# formatting programs. +# +set c_files +set f_files +set all no + +if test "$argv[1]" = "--all" + set all yes + set -e argv[1] +end + +if set -q argv[1] + echo "Unexpected arguments: '$argv'" + exit 1 +end + +if test $all = yes + set c_files src/*.h src/*.cpp + set f_files ***.fish +else + # We haven't been asked to reformat all the source. If there are uncommitted + # changes reformat those, else reformat the files in the most recent commit. + set pending (git status --porcelain --short --untracked-files=all | sed -e 's/^ *//') + if count $pending > /dev/null + # There are pending changes so lint those files. + for arg in $pending + set files $files (string split -m 1 ' ' $arg)[2] + end + else + # No pending changes so lint the files in the most recent commit. + set files (git show --name-only --pretty=oneline head | tail --lines=+2) + end + + # Extract just the C/C++ files. + set c_files (string match -r '^.*\.(?:c|cpp|h)$' -- $files) + # Extract just the fish files. + set f_files (string match -r '^.*\.fish$' -- $files) +end + +# Run the C++ reformatter if we have any C++ files. +if set -q c_files[1] + if type -q clang-format + echo + echo ======================================== + echo Running clang-format + echo ======================================== + for file in $c_files + clang-format $file > $file.new + if cmp --quiet $file $file.new + echo $file was correctly formatted + rm $file.new + else + echo $file was NOT correctly formatted + mv $file.new $file + end + end + else + echo + echo 'WARNING: Cannot find clang-format command' + echo + end +end + +# Run the fish reformatter if we have any fish files. +if set -q f_files[1] + if not type -q fish_indent + make fish_indent + set PATH . $PATH + end + echo + echo ======================================== + echo Running fish_indent + echo ======================================== + for file in $f_files + fish_indent < $file > $file.new + if cmp --quiet $file $file.new + echo $file was correctly formatted + rm $file.new + else + echo $file was NOT correctly formatted + mv $file.new $file + end + end +end -- cgit v1.2.3