Re: [PATCH 8/8] check-whitespace: detect if no base_commit is provided

From: Chris Torek <chris.torek@gmail.com>
Date: Jul 8, 2024, 10:18 UTC
Message-ID: <CAPx1GvfkC0rv_8_q5_HfduhuKX4csO+sBZiZqV+Sh+Q0dV8+4w@mail.gmail.com>
In-Reply-To: <20240708092317.267915-9-karthik.188@gmail.com>

On Mon, Jul 8, 2024 at 2:24 AM Karthik Nayak <karthik.188@gmail.com> wrote:

Show 11 quoted lines

> diff --git a/ci/check-whitespace.sh b/ci/check-whitespace.sh
> index db399097a5..ab023f9519 100755
> --- a/ci/check-whitespace.sh
> +++ b/ci/check-whitespace.sh
> @@ -9,12 +9,19 @@ baseCommit=$1
>  outputFile=$2
>  url=$3
>
> -if test "$#" -ne 1 && test "$#" -ne 3
> +if { test "$#" -ne 1 && test "$#" -ne 3; } || test -z "$1"
>  then

The braces `{ ... }` here are unnecessary as `&&`will bind first (sh and bash give both operators the same precedence but then use left associativity). Dropping them drops the need for the semicolon. Of course some might prefer this to be explicit.

The `test` command has AND and OR operators of its own, which give `-a` (AND) higher precedence than `-o` (OR). In addition, `$#` can only expand to an integer value, so quotes are not required, and the whole thing can be written as:

    if test $# -ne 1 -a $# -ne 3 -o -z "$1"

(which is what I would do myself, unless I wanted a separate error message for an empty "$1"). It's fine as is though.

Chris