Merge pull request #37421 from akien-mga/python-format-black

SCons: Format buildsystem files with psf/black

7 files changed, 221 insertions, 33 deletions

diff --git a/misc/hooks/README.md b/misc/hooks/README.md
index b18ba7df38..dad5300a09 100644
--- a/misc/hooks/README.md
+++ b/misc/hooks/README.md
@@ -5,16 +5,22 @@ contributors to make sure they comply with our requirements.
 
 ## List of hooks
 
-- Pre-commit hook for clang-format: Applies clang-format to the staged files
-  before accepting a commit; blocks the commit and generates a patch if the
-  style is not respected.
-  Should work on Linux and macOS. You may need to edit the file if your
-  clang-format binary is not in the `$PATH`, or if you want to enable colored
-  output with pygmentize.
-- Pre-commit hook for makerst: Checks the class reference syntax using `makerst.py`.
-  Should work on Linux and macOS.
+- Pre-commit hook for `clang-format`: Applies `clang-format` to the staged
+  files before accepting a commit; blocks the commit and generates a patch if
+  the style is not respected.
+  You may need to edit the file if your `clang-format` binary is not in the
+  `PATH`, or if you want to enable colored output with `pygmentize`.
+- Pre-commit hook for `black`: Applies `black` to the staged Python files
+  before accepting a commit.
+- Pre-commit hook for `makerst`: Checks the class reference syntax using
+  `makerst.py`.
 
 ## Installation
 
-Copy all the files from this folder into your `.git/hooks` folder, and make sure
-the hooks and helper scripts are executable.
+Copy all the files from this folder into your `.git/hooks` folder, and make
+sure the hooks and helper scripts are executable.
+
+The hooks rely on bash scripts and tools which should be in the system `PATH`,
+so they should work out of the box on Linux/macOS, and might work on Windows
+when using `git-bash.exe` with `clang-format`, Python, `black`, etc. in the
+`PATH`.
diff --git a/misc/hooks/pre-commit b/misc/hooks/pre-commit
index 36e9935785..40cb00253b 100755
--- a/misc/hooks/pre-commit
+++ b/misc/hooks/pre-commit
@@ -14,7 +14,7 @@
 # as this script. Hooks should return 0 if successful and nonzero to cancel the
 # commit. They are executed in the order in which they are listed.
 #HOOKS="pre-commit-compile pre-commit-uncrustify"
-HOOKS="pre-commit-clang-format pre-commit-makerst"
+HOOKS="pre-commit-clang-format pre-commit-black pre-commit-makerst"
 ###########################################################
 # There should be no need to change anything below this line.
 
diff --git a/misc/hooks/pre-commit-black b/misc/hooks/pre-commit-black
new file mode 100755
index 0000000000..2dcc2e8cf1
--- /dev/null
+++ b/misc/hooks/pre-commit-black
@@ -0,0 +1,132 @@
+#!/usr/bin/env bash
+
+# git pre-commit hook that runs a black stylecheck.
+# Based on pre-commit-clang-format.
+
+##################################################################
+# SETTINGS
+# Set path to black binary.
+BLACK=`which black`
+BLACK_OPTIONS="-l 120"
+
+# Remove any older patches from previous commits. Set to true or false.
+DELETE_OLD_PATCHES=false
+
+# File types to parse.
+FILE_NAMES="SConstruct SCsub"
+FILE_EXTS="py"
+
+# Use pygmentize instead of cat to parse diff with highlighting.
+# Install it with `pip install pygments` (Linux) or `easy_install Pygments` (Mac)
+PYGMENTIZE=`which pygmentize`
+if [ ! -z "$PYGMENTIZE" ]; then
+  READER="pygmentize -l diff"
+else
+  READER=cat
+fi
+
+##################################################################
+# There should be no need to change anything below this line.
+
+. "$(dirname -- "$0")/canonicalize_filename.sh"
+
+# exit on error
+set -e
+
+# check whether the given file matches any of the set extensions
+matches_name_or_extension() {
+    local filename=$(basename "$1")
+    local extension=".${filename##*.}"
+
+    for name in $FILE_NAMES; do [[ "$name" == "$filename" ]] && return 0; done
+    for ext in $FILE_EXTS; do [[ "$ext" == "$extension" ]] && return 0; done
+
+    return 1
+}
+
+# necessary check for initial commit
+if git rev-parse --verify HEAD >/dev/null 2>&1 ; then
+    against=HEAD
+else
+    # Initial commit: diff against an empty tree object
+    against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+fi
+
+if [ ! -x "$BLACK" ] ; then
+    printf "Error: black executable not found.\n"
+    printf "Set the correct path in $(canonicalize_filename "$0").\n"
+    exit 1
+fi
+
+# create a random filename to store our generated patch
+prefix="pre-commit-black"
+suffix="$(date +%s)"
+patch="/tmp/$prefix-$suffix.patch"
+
+# clean up any older black patches
+$DELETE_OLD_PATCHES && rm -f /tmp/$prefix*.patch
+
+# create one patch containing all changes to the files
+git diff-index --cached --diff-filter=ACMR --name-only $against -- | while read file;
+do
+    # ignore thirdparty files
+    if grep -q "thirdparty" <<< $file; then
+        continue;
+    fi
+
+    # ignore file if not one of the names or extensions we handle
+    if ! matches_name_or_extension "$file"; then
+        continue;
+    fi
+
+    # format our file with black, create a patch with diff and append it to our $patch
+    # The sed call is necessary to transform the patch from
+    #    --- $file timestamp
+    #    +++ $file timestamp
+    # to both lines working on the same file and having a/ and b/ prefix.
+    # Else it can not be applied with 'git apply'.
+    "$BLACK" "$BLACK_OPTIONS" --diff "$file" | \
+        sed -e "1s|--- |--- a/|" -e "2s|+++ |+++ b/|" >> "$patch"
+done
+
+# if no patch has been generated all is ok, clean up the file stub and exit
+if [ ! -s "$patch" ] ; then
+    printf "Files in this commit comply with the black formatter rules.\n"
+    rm -f "$patch"
+    exit 0
+fi
+
+# a patch has been created, notify the user and exit
+printf "\nThe following differences were found between the code to commit "
+printf "and the black formatter rules:\n\n"
+$READER "$patch"
+printf "\n"
+
+# Allows us to read user input below, assigns stdin to keyboard
+exec < /dev/tty
+
+while true; do
+    read -p "Do you want to apply that patch (Y - Apply, N - Do not apply, S - Apply and stage files)? [Y/N/S] " yn
+    case $yn in
+        [Yy] ) git apply $patch;
+        printf "The patch was applied. You can now stage the changes and commit again.\n\n";
+        break
+        ;;
+        [Nn] ) printf "\nYou can apply these changes with:\n  git apply $patch\n";
+        printf "(may need to be called from the root directory of your repository)\n";
+        printf "Aborting commit. Apply changes and commit again or skip checking with";
+        printf " --no-verify (not recommended).\n\n";
+        break
+        ;;
+        [Ss] ) git apply $patch;
+        git diff-index --cached --diff-filter=ACMR --name-only $against -- | while read file;
+        do git add $file;
+        done
+        printf "The patch was applied and the changed files staged. You can now commit.\n\n";
+        break
+        ;;
+        * ) echo "Please answer yes or no."
+        ;;
+    esac
+done
+exit 1 # we don't commit in any case
diff --git a/misc/hooks/pre-commit-clang-format b/misc/hooks/pre-commit-clang-format
index e309233a8b..c5cf4ecbb1 100755
--- a/misc/hooks/pre-commit-clang-format
+++ b/misc/hooks/pre-commit-clang-format
@@ -15,28 +15,28 @@
 
 ##################################################################
 # SETTINGS
-# Set path to clang-format binary
-# CLANG_FORMAT="/usr/bin/clang-format"
+# Set path to clang-format binary.
 CLANG_FORMAT=`which clang-format`
 
 # Remove any older patches from previous commits. Set to true or false.
-# DELETE_OLD_PATCHES=false
 DELETE_OLD_PATCHES=false
 
 # Only parse files with the extensions in FILE_EXTS. Set to true or false.
 # If false every changed file in the commit will be parsed with clang-format.
 # If true only files matching one of the extensions are parsed with clang-format.
-# PARSE_EXTS=true
 PARSE_EXTS=true
 
 # File types to parse. Only effective when PARSE_EXTS is true.
-# FILE_EXTS=".c .h .cpp .hpp"
 FILE_EXTS=".c .h .cpp .hpp .cc .hh .cxx .m .mm .inc .java .glsl"
 
 # Use pygmentize instead of cat to parse diff with highlighting.
 # Install it with `pip install pygments` (Linux) or `easy_install Pygments` (Mac)
-# READER="pygmentize -l diff"
-READER=cat
+PYGMENTIZE=`which pygmentize`
+if [ ! -z "$PYGMENTIZE" ]; then
+  READER="pygmentize -l diff"
+else
+  READER=cat
+fi
 
 ##################################################################
 # There should be no need to change anything below this line.
diff --git a/misc/scripts/fix_headers.py b/misc/scripts/fix_headers.py
index f0038a8351..7af97eec4b 100755
--- a/misc/scripts/fix_headers.py
+++ b/misc/scripts/fix_headers.py
@@ -37,24 +37,24 @@ files = open("files", "r")
 
 fname = files.readline()
 
-while (fname != ""):
+while fname != "":
 
     # Handle replacing $filename with actual filename and keep alignment
     fsingle = fname.strip()
-    if (fsingle.find("/") != -1):
-        fsingle = fsingle[fsingle.rfind("/") + 1:]
+    if fsingle.find("/") != -1:
+        fsingle = fsingle[fsingle.rfind("/") + 1 :]
     rep_fl = "$filename"
     rep_fi = fsingle
     len_fl = len(rep_fl)
     len_fi = len(rep_fi)
     # Pad with spaces to keep alignment
-    if (len_fi < len_fl):
+    if len_fi < len_fl:
         for x in range(len_fl - len_fi):
             rep_fi += " "
-    elif (len_fl < len_fi):
+    elif len_fl < len_fi:
         for x in range(len_fi - len_fl):
             rep_fl += " "
-    if (header.find(rep_fl) != -1):
+    if header.find(rep_fl) != -1:
         text = header.replace(rep_fl, rep_fi)
     else:
         text = header.replace("$filename", fsingle)
@@ -71,21 +71,21 @@ while (fname != ""):
     line = fileread.readline()
     header_done = False
 
-    while (line.strip() == ""): # Skip empty lines at the top
+    while line.strip() == "":  # Skip empty lines at the top
         line = fileread.readline()
 
-    if (line.find("/**********") == -1): # Godot header starts this way
+    if line.find("/**********") == -1:  # Godot header starts this way
         # Maybe starting with a non-Godot comment, abort header magic
         header_done = True
 
-    while (not header_done): # Handle header now
-        if (line.find("/*") != 0): # No more starting with a comment
+    while not header_done:  # Handle header now
+        if line.find("/*") != 0:  # No more starting with a comment
             header_done = True
-            if (line.strip() != ""):
+            if line.strip() != "":
                 text += line
         line = fileread.readline()
 
-    while (line != ""): # Dump everything until EOF
+    while line != "":  # Dump everything until EOF
         text += line
         line = fileread.readline()
 
diff --git a/misc/travis/black-format.sh b/misc/travis/black-format.sh
new file mode 100755
index 0000000000..75b153f6bb
--- /dev/null
+++ b/misc/travis/black-format.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+BLACK=black
+BLACK_OPTIONS="-l 120"
+
+if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
+    # Travis only clones the PR branch and uses its HEAD commit as detached HEAD,
+    # so it's problematic when we want an exact commit range for format checks.
+    # We fetch upstream to ensure that we have the proper references to resolve.
+    # Ideally we would use $TRAVIS_COMMIT_RANGE but it doesn't play well with PR
+    # updates, as it only includes changes since the previous state of the PR.
+    if [ -z "$(git remote | grep upstream)" ]; then
+      git remote add upstream https://github.com/godotengine/godot \
+          --no-tags -f -t $TRAVIS_BRANCH
+    fi
+    RANGE="upstream/$TRAVIS_BRANCH HEAD"
+else
+    # Test only the last commit, since $TRAVIS_COMMIT_RANGE wouldn't support
+    # force pushes.
+    RANGE=HEAD
+fi
+
+FILES=$(git diff-tree --no-commit-id --name-only -r $RANGE | grep -v thirdparty/| grep -E "(SConstruct|SCsub|\.py)$")
+echo "Checking files:\n$FILES"
+
+# create a random filename to store our generated patch
+prefix="static-check-black"
+suffix="$(date +%s)"
+patch="/tmp/$prefix-$suffix.patch"
+
+for file in $FILES; do
+    "$BLACK" "$BLACK_OPTIONS" --diff "$file" | \
+        sed -e "1s|--- |--- a/|" -e "2s|+++ |+++ b/|" >> "$patch"
+done
+
+# if no patch has been generated all is ok, clean up the file stub and exit
+if [ ! -s "$patch" ] ; then
+    printf "Files in this commit comply with the black formatting rules.\n"
+    rm -f "$patch"
+    exit 0
+fi
+
+# a patch has been created, notify the user and exit
+printf "\n*** The following differences were found between the code to commit "
+printf "and the black formatting rules:\n\n"
+pygmentize -l diff "$patch"
+printf "\n*** Aborting, please fix your commit(s) with 'git commit --amend' or 'git rebase -i <hash>'\n"
+exit 1
diff --git a/misc/travis/clang-format.sh b/misc/travis/clang-format.sh
index a6585578e1..c917744ece 100755
--- a/misc/travis/clang-format.sh
+++ b/misc/travis/clang-format.sh
@@ -8,8 +8,10 @@ if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
     # We fetch upstream to ensure that we have the proper references to resolve.
     # Ideally we would use $TRAVIS_COMMIT_RANGE but it doesn't play well with PR
     # updates, as it only includes changes since the previous state of the PR.
-    git remote add upstream https://github.com/godotengine/godot \
-        --no-tags -f -t $TRAVIS_BRANCH
+    if [ -z "$(git remote | grep upstream)" ]; then
+      git remote add upstream https://github.com/godotengine/godot \
+          --no-tags -f -t $TRAVIS_BRANCH
+    fi
     RANGE="upstream/$TRAVIS_BRANCH HEAD"
 else
     # Test only the last commit, since $TRAVIS_COMMIT_RANGE wouldn't support
@@ -41,6 +43,6 @@ fi
 # a patch has been created, notify the user and exit
 printf "\n*** The following differences were found between the code to commit "
 printf "and the clang-format rules:\n\n"
-cat "$patch"
+pygmentize -l diff "$patch"
 printf "\n*** Aborting, please fix your commit(s) with 'git commit --amend' or 'git rebase -i <hash>'\n"
 exit 1