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
This commit is contained in:
Kurtis Rader 2016-04-01 20:48:11 -07:00
parent a4642f141f
commit fd1b7ba529
5 changed files with 174 additions and 90 deletions

8
.clang-format Normal file
View file

@ -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

View file

@ -1,13 +1,57 @@
# 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.
## 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
```
make style
```
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
```
make style-all
```
## Fish Script Style Guide
Fish scripts such as those in the *share/functions* and *tests* directories should be formatted using the `fish_indent` command.
Function names should be all lowercase with undescores separating words. Private functions should begin with an underscore. The first word should be `fish` if the function is unique to fish.
The first word of global variable names should generally be `fish` for public vars or `_fish` for private vars to minimize the possibility of name clashes with user defined vars.
## C++ Style Guide
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.
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.
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. Always attach braces to the surrounding context.
1. Indent with spaces, not tabs and use four spaces per indent.
## Installing the Required Tools
### Installing the Linting Tools
To install the lint checkers on Mac OS X using HomeBrew:
```
@ -24,85 +68,24 @@ sudo apt-get install oclint
sudo apt-get install cppcheck
```
## Fish Script Style Guide
### Installing the Reformatting Tools
Fish scripts such as those in the *share/functions* and *tests* directories should be formatted using the `fish_indent` command.
To install the reformatting tool on Mac OS X using HomeBrew:
Function names should be all lowercase with undescores separating words. Private functions should begin with an underscore. The first word should be `fish` if the function is unique to fish.
```
brew install clang-format
```
The first word of global variable names should generally be `fish` for public vars or `_fish` for private vars to minimize the possibility of name clashes with user defined vars.
To install the reformatting tool on Linux distros that use Apt:
## C++ Style Guide
```
apt-cache search clang-format
```
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.
That will list the versions available. Pick the newest one available (3.6 for Ubuntu 14.04 as I write this) and install it:
1. All names in code should be `small_snake_case`. No Hungarian notation is used. Classes and structs names should be followed by `_t`.
```
sudo apt-get install clang-format-3.6
sudo ln -s /usr/bin/clang-format-3.6 /usr/bin/clang-format
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
};
void func()
{
// code
}
if (...)
{
// code
}
// ✗:
void func() {
// code
}
1. Put space after `if`, `while` and `for` before conditions.
// ✔:
if () {}
// ✗:
if() {}
1. Put spaces before and after operators excluding increment and decrement;
// ✔:
int a = 1 + 2 * 3;
a++;
// ✗:
int a=1+2*3;
a ++;
1. Never put spaces between function name and parameters list.
// ✔:
func(args);
// ✗:
func (args);
1. Never put spaces after `(` and before `)`.
1. Always put space after comma and semicolon.
// ✔:
func(arg1, arg2);
for (int i = 0; i < LENGTH; i++) {}
// ✗:
func(arg1,arg2);
for (int i = 0;i<LENGTH;i++) {}
```

View file

@ -872,13 +872,21 @@ iwyu:
_iwyu: clean $(PROGRAMS)
.PHONY: iwyu _iwyu
# Lint the code.
# Lint the code. This only deals with C++ files.
lint:
build_tools/lint.fish $(CXX) $(CXXFLAGS)
lint-all:
build_tools/lint.fish $(CXX) --all $(CXXFLAGS)
.PHONY: lint lint-all
# Run the code through the style refomatter. This handles both C++ files and
# fish scripts (*.fish).
style:
build_tools/style.fish
style-all:
build_tools/style.fish --all
.PHONY: lint lint-all
#
# Cleanup targets
#

21
build_tools/lint.fish Executable file → Normal file
View file

@ -13,7 +13,6 @@ set -e argv[1]
if test "$argv[1]" = "--all"
set all yes
set c_files src/
set cppchecks "$cppchecks,unusedFunction"
set -e argv[1]
end
@ -30,7 +29,9 @@ if test (uname -m) = "x86_64"
set cppcheck_args -D__x86_64__ -D__LP64__ $cppcheck_args
end
if test $all = no
if test $all = yes
set c_files src/*.cpp
else
# We haven't been asked to lint all the source. If there are uncommitted
# changes lint those, else lint the files in the most recent commit.
set pending (git status --porcelain --short --untracked-files=all | sed -e 's/^ *//')
@ -44,10 +45,8 @@ if test $all = no
set files (git show --porcelain --name-only --pretty=oneline head | tail --lines=+2)
end
# Filter out the non-C/C++ files.
# Extract just the C/C++ files.
set c_files (string match -r '.*\.c(?:pp)?$' -- $files)
else
set c_files src/*.cpp
end
# We now have a list of files to check so run the linters.
@ -61,10 +60,7 @@ if set -q c_files[1]
# IMHO, writes its diagnostic messages to stderr. Anyone running
# this who wants to capture its output will expect those messages to be
# written to stdout.
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
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

90
build_tools/style.fish Executable file
View file

@ -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