From d702d8073e2572a19c806fa53e484a25863f6df4 Mon Sep 17 00:00:00 2001 From: Kai Liu Date: Tue, 14 Jan 2014 00:39:42 -0800 Subject: [PATCH] A script that automatically reformat affected lines Summary: Added a script that reformat only the affected lines in a given diff. I planned to make that file as pre-commit hook but looks it's a little bit more difficult than I thought. Since I don't want to spend too much time on this task right now, I eventually added a "make command" to achieve this with a few additional key strokes. Also make the clang-format solely inherited from Google's style -- there are still debates on some of the style issues, but we can address them later once we reach a consensus. Test Plan: Did some ugly format change and ran "make format", all affected lines are formatted as expected. Reviewers: igor, sdong, haobo CC: leveldb Differential Revision: https://reviews.facebook.net/D15147 --- .clang-format | 42 ------------------- Makefile | 11 ++++- build_tools/format-diff.sh | 83 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 43 deletions(-) create mode 100755 build_tools/format-diff.sh diff --git a/.clang-format b/.clang-format index a1e9a48e40..7c279811ac 100644 --- a/.clang-format +++ b/.clang-format @@ -2,46 +2,4 @@ # http://clang.llvm.org/docs/ClangFormatStyleOptions.html --- BasedOnStyle: Google -AccessModifierOffset: -1 -ConstructorInitializerIndentWidth: 4 -AlignEscapedNewlinesLeft: true -AlignTrailingComments: true -AllowAllParametersOfDeclarationOnNextLine: true -AllowShortIfStatementsOnASingleLine: false -AllowShortLoopsOnASingleLine: false -AlwaysBreakTemplateDeclarations: true -AlwaysBreakBeforeMultilineStrings: true -BreakBeforeBinaryOperators: false -BreakConstructorInitializersBeforeComma: false -BinPackParameters: false -ColumnLimit: 80 -ConstructorInitializerAllOnOneLineOrOnePerLine: true -DerivePointerBinding: true -ExperimentalAutoDetectBinPacking: true -IndentCaseLabels: false -MaxEmptyLinesToKeep: 1 -NamespaceIndentation: None -ObjCSpaceBeforeProtocolList: false -PenaltyBreakBeforeFirstCallParameter: 10 -PenaltyBreakComment: 60 -PenaltyBreakString: 1000 -PenaltyBreakFirstLessLess: 20 -PenaltyExcessCharacter: 1000000 -PenaltyReturnTypeOnItsOwnLine: 200 -PointerBindsToType: true -SpacesBeforeTrailingComments: 2 -Cpp11BracedListStyle: true -Standard: Cpp11 -IndentWidth: 2 -TabWidth: 8 -UseTab: Never -BreakBeforeBraces: Attach -IndentFunctionDeclarationAfterType: false -SpacesInParentheses: false -SpacesInAngles: false -SpaceInEmptyParentheses: false -SpacesInCStyleCastParentheses: false -SpaceAfterControlStatementKeyword: true -SpaceBeforeAssignmentOperators: true -ContinuationIndentWidth: 4 ... diff --git a/Makefile b/Makefile index 5170ac54a9..ebf7b96fe3 100644 --- a/Makefile +++ b/Makefile @@ -135,7 +135,7 @@ endif # PLATFORM_SHARED_EXT all: $(LIBRARY) $(PROGRAMS) $(SHARED) .PHONY: blackbox_crash_test check clean coverage crash_test ldb_tests \ - release tags valgrind_check whitebox_crash_test + release tags valgrind_check whitebox_crash_test format release: $(MAKE) clean @@ -196,6 +196,9 @@ tags: ctags * -R cscope -b `find . -name '*.cc'` `find . -name '*.h'` +format: + build_tools/format-diff.sh + # --------------------------------------------------------------------------- # Unit tests and tools # --------------------------------------------------------------------------- @@ -411,6 +414,12 @@ DEPFILES = $(filter-out util/build_version.d,$(SOURCES:.cc=.d)) depend: $(DEPFILES) +# if the make goal is either "clean" or "format", we shouldn't +# try to import the *.d files. +# TODO(kailiu) The unfamiliarity of Make's conditions leads to the ugly +# working solution. ifneq ($(MAKECMDGOALS),clean) +ifneq ($(MAKECMDGOALS),format) -include $(DEPFILES) endif +endif diff --git a/build_tools/format-diff.sh b/build_tools/format-diff.sh new file mode 100755 index 0000000000..758135c9f8 --- /dev/null +++ b/build_tools/format-diff.sh @@ -0,0 +1,83 @@ +#!/bin/bash +set -e +# If clang_format_diff.py command is not specfied, we assume we are able to +# access directly without any path. +if [ -z $CLANG_FORMAT_DIFF ] +then +CLANG_FORMAT_DIFF="clang-format-diff.py" +fi + +# Check clang-format-diff.py +if ! which $CLANG_FORMAT_DIFF &> /dev/null +then + echo "You didn't have clang-format-diff.py available in your computer!" + echo "You can download it by running: " + echo " curl https://fburl.com/clang-format-diff" + exit 128 +fi + +# Check argparse, a library that clang-format-diff.py requires. +python 2>/dev/null << EOF +import argparse +EOF + +if [ "$?" != 0 ] +then + echo "To run clang-format-diff.py, we'll need the library "argparse" to be" + echo "installed. You can try either of the follow ways to install it:" + echo " 1. Manually download argparse: https://pypi.python.org/pypi/argparse" + echo " 2. easy_install argparse (if you have easy_install)" + echo " 3. pip install argparse (if you have pip)" + exit 129 +fi + +# TODO(kailiu) following work is not complete since we still need to figure +# out how to add the modified files done pre-commit hook to git's commit index. +# +# Check if this script has already been added to pre-commit hook. +# Will suggest user to add this script to pre-commit hook if their pre-commit +# is empty. +# PRE_COMMIT_SCRIPT_PATH="`git rev-parse --show-toplevel`/.git/hooks/pre-commit" +# if ! ls $PRE_COMMIT_SCRIPT_PATH &> /dev/null +# then +# echo "Would you like to add this script to pre-commit hook, which will do " +# echo -n "the format check for all the affected lines before you check in (y/n):" +# read add_to_hook +# if [ "$add_to_hook" == "y" ] +# then +# ln -s `git rev-parse --show-toplevel`/build_tools/format-diff.sh $PRE_COMMIT_SCRIPT_PATH +# fi +# fi + +# Check the format of recently changed lines, +diffs=$(git diff -U0 HEAD^ | $CLANG_FORMAT_DIFF -p 1) + +if [ -z "$diffs" ] +then + echo "Nothing needs to be reformatted!" + exit 0 +fi + +# Highlight the insertion/deletion from the clang-format-diff.py's output +COLOR_END="\033[0m" +COLOR_RED="\033[0;31m" +COLOR_GREEN="\033[0;32m" + +echo -e "Detect lines that doesn't follow the format rules:\r" +# Add the color to the diff. lines added will be green; lines removed will be red. +echo "$diffs" | + sed -e "s/\(^-.*$\)/`echo -e \"$COLOR_RED\1$COLOR_END\"`/" | + sed -e "s/\(^+.*$\)/`echo -e \"$COLOR_GREEN\1$COLOR_END\"`/" +echo -e "Would you like to fix the format automatically (y/n): \c" + +# Make sure under any mode, we can read user input. +exec < /dev/tty +read to_fix + +if [ "$to_fix" != "y" ] +then + exit 1 +fi + +# Do in-place format adjustment. +git diff -U0 HEAD^ | $CLANG_FORMAT_DIFF -i -p 1