From fe5adc809aeaafc58c482cf43638b411a29ca8d6 Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Mon, 2 Oct 2023 14:51:36 -0700 Subject: [PATCH 1/2] Use format-incremental from cirq --- check/format-incremental | 127 ++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 63 deletions(-) diff --git a/check/format-incremental b/check/format-incremental index dec211a92..b163cadd9 100755 --- a/check/format-incremental +++ b/check/format-incremental @@ -1,22 +1,23 @@ #!/usr/bin/env bash ################################################################################ -# Formats lines of python code that have been modified. +# Formats python files that have been modified. # # Usage: -# check/format-incremental [BASE_REVISION] [--apply] +# check/format-incremental [BASE_REVISION] [--apply] [--all] # -# Without '--apply', the diff that would be applied is printed and the exit -# status is 1 if there are any changes or else 0 if no changes are needed. +# By default, the script analyzes python files that have changed relative to the +# base revision and determines whether they need to be formatted. If any changes +# are needed, it prints the diff and exits with code 1, otherwise it exits with +# code 0. # -# With '--apply', the exit status is 0 and the changed files are actually -# reformated. +# With '--apply', reformats the files instead of printing the diff and exits +# with code 0. # -# Note that sometimes yapf will format unchanged lines. In particular, it -# completely ignores the changed line ranges when fixing indentation. +# With '--all', analyzes all python files, instead of only changed files. # # You can specify a base git revision to compare against (i.e. to use when -# determining whether or not a line is considered to have "changed"). For +# determining whether or not a file is considered to have "changed"). For # example, you can compare against 'origin/master' or 'HEAD~1'. # # If you don't specify a base revision, the following defaults will be tried, in @@ -30,82 +31,82 @@ ################################################################################ # Get the working directory to the repo root. -cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -cd "$(git rev-parse --show-toplevel)" +thisdir="$(dirname "${BASH_SOURCE[0]}")" || exit $? +topdir="$(git -C "${thisdir}" rev-parse --show-toplevel)" || exit $? +cd "${topdir}" || exit $? + # Parse arguments. only_print=1 +only_changed=1 rev="" -for arg in $@; do +for arg in "$@"; do if [[ "${arg}" == "--apply" ]]; then only_print=0 + elif [[ "${arg}" == "--all" ]]; then + only_changed=0 elif [ -z "${rev}" ]; then - if [ "$(git cat-file -t ${arg} 2> /dev/null)" != "commit" ]; then + if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2 exit 1 fi rev="${arg}" else - echo -e "\033[31mToo many arguments. Expected [revision] [--apply].\033[0m" >&2 + echo -e "\033[31mToo many arguments. Expected [revision] [--apply] [--all].\033[0m" >&2 exit 1 fi done -# Figure out which branch to compare against. -if [ -z "${rev}" ]; then - if [ "$(git cat-file -t upstream/master 2> /dev/null)" == "commit" ]; then - rev=upstream/master - elif [ "$(git cat-file -t origin/master 2> /dev/null)" == "commit" ]; then - rev=origin/master - elif [ "$(git cat-file -t master 2> /dev/null)" == "commit" ]; then - rev=master +typeset -a format_files +if (( only_changed == 1 )); then + # Figure out which branch to compare against. + if [ -z "${rev}" ]; then + if [ "$(git cat-file -t upstream/master 2> /dev/null)" == "commit" ]; then + rev=upstream/master + elif [ "$(git cat-file -t origin/master 2> /dev/null)" == "commit" ]; then + rev=origin/master + elif [ "$(git cat-file -t master 2> /dev/null)" == "commit" ]; then + rev=master + else + echo -e "\033[31mNo default revision found to compare against. Argument #1 must be what to diff against (e.g. 'origin/master' or 'HEAD~1').\033[0m" >&2 + exit 1 + fi + fi + base="$(git merge-base "${rev}" HEAD)" + if [ "$(git rev-parse "${rev}")" == "${base}" ]; then + echo -e "Comparing against revision '${rev}'." >&2 else - echo -e "\033[31mNo default revision found to compare against. Argument #1 must be what to diff against (e.g. 'origin/master' or 'HEAD~1').\033[0m" >&2 - exit 1 + echo -e "Comparing against revision '${rev}' (merge base ${base})." >&2 + rev="${base}" fi -fi -base="$(git merge-base ${rev} HEAD)" -if [ "$(git rev-parse ${rev})" == "${base}" ]; then - echo -e "Comparing against revision '${rev}'." >&2 + + # Get the modified, added and moved python files. + IFS=$'\n' read -r -d '' -a format_files < \ + <(git diff --name-only --diff-filter=MAR "${rev}" -- '*.py' ':(exclude)*_pb2.py') else - echo -e "Comparing against revision '${rev}' (merge base ${base})." >&2 - rev="${base}" + echo -e "Formatting all python files." >&2 + IFS=$'\n' read -r -d '' -a format_files < \ + <(git ls-files '*.py' ':(exclude)*_pb2.py') fi -# Get the _test version of changed python files. -needed_changes=0 -changed_files=$(git diff --name-only ${rev} -- | grep "\.py$" | grep -v "_pb2\.py$") -esc=$(printf '\033') -for changed_file in ${changed_files}; do - # Extract changed line ranges from diff output. - changed_line_ranges=$( \ - git diff --unified=0 "${rev}" -- "${changed_file}" \ - | perl -ne 'chomp(); if (/@@ -\d+(,\d+)? \+(\d+)(,)?(\d+)? @@/) {$end=$2+($4 or 1)-1; print "--lines=$2-$end "}' \ - ) +if (( ${#format_files[@]} == 0 )); then + echo -e "\033[32mNo files to format\033[0m." + exit 0 +fi - if [[ "${changed_line_ranges}" != "--lines=0-0 " ]]; then - # Do the formatting. - results=$(yapf --style=google --diff "${changed_file}" ${changed_line_ranges}) +BLACKVERSION="$(black --version)" - # Print colorized error messages. - if [ ! -z "${results}" ]; then - needed_changes=1 - if (( only_print == 0 )); then - $(yapf --style=google --in-place "${changed_file}" ${changed_line_ranges}) - else - echo -e "\n\033[31mChanges in ${changed_file} require formatting:\033[0m\n${results}" \ - | sed "s/^\(+ .*\)$/${esc}[32m\\1${esc}[0m/" \ - | sed "s/^\(- .*\)$/${esc}[31m\\1${esc}[0m/" - fi - fi - fi -done +echo "Running the black formatter... (version: $BLACKVERSION)" -if (( needed_changes == 0 )); then - echo -e "\033[32mNo formatting needed on changed lines\033[0m." -elif (( only_print == 1 )); then - echo -e "\033[31mSome formatting needed on changed lines\033[0m." - exit 1 -else - echo -e "\033[33mReformatted changed lines\033[0m." +args=("--color") +if (( only_print == 1 )); then + args+=("--check" "--diff") +fi + +black "${args[@]}" "${format_files[@]}" +BLACKSTATUS=$? + +if [[ "$BLACKSTATUS" != "0" ]]; then + exit 1 fi +exit 0 From c2e5f98126afdd44ada09c58fed6aa3fe3aa3499 Mon Sep 17 00:00:00 2001 From: Matthew Harrigan Date: Mon, 2 Oct 2023 14:54:04 -0700 Subject: [PATCH 2/2] deps --- dev_tools/requirements/deps/format.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev_tools/requirements/deps/format.txt b/dev_tools/requirements/deps/format.txt index a9b79a592..4cbbfb881 100644 --- a/dev_tools/requirements/deps/format.txt +++ b/dev_tools/requirements/deps/format.txt @@ -1 +1,2 @@ -yapf~=0.27.0 +# Make sure this matches cirq/dev_tools/requirements/deps/format.txt +black==23.3.0