Re: [PATCH v3 1/2] t/lib-read-tree-m-3way: modernize style

[Christian Couder <christian.couder@gmail.com>]

On Fri, Feb 4, 2022 at 6:00 AM Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> wrote:
>
> Many invocations of the test_expect_success command in this
> file are written in old style where the command, an optional
> prerequisite, and the test title are written on separate
> lines, and the executable script string begins on its own
> line, and these lines are pasted together with backslashes
> as necessary.

It's not very clear here if "these lines" means only the separate
lines with the command, an optional prerequisite, and the test title,
or if it also means the first (or maybe many) line(s) of the
executable script string.

> An invocation of the test_expect_success command in modern
> test scripts however writes the prerequisite and the title
> on the same line as the test_expect_success command itself,
> and ends the line with a single quote that begins the
> executable script string.

It could also be 'test_expect_failure' instead of 'test_expect_success'.

> Update the style for uniformity.
>
> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>

>  for p in M? Z/M?
>  do
>      echo This is modified $p in the branch A. >$p
> -    test_expect_success \
> -       'change in branch A (modification)' \
> -        "git update-index $p"
> +    test_expect_success 'change in branch A (modification)' '
> +        git update-index $p
> +    '

The above is not just about moving single quotes from one line to
another, but it changes some double quotes to single quotes, which
means that $p might not be interpreted in the same way. This is not
just a style issue and it should be explained in the commit message
why it's ok to make this change.

>  done
>
>  for p in AN AA Z/AN Z/AA
>  do
>      echo This is added $p in the branch A. >$p
> -    test_expect_success \
> -       'change in branch A (addition)' \
> -       "git update-index --add $p"
> +    test_expect_success 'change in branch A (addition)' '
> +           git update-index --add $p
> +    '

Here also some double quotes are changed into single quotes.

>  done

>  to_remove=$(echo ?D Z/?D)
>  rm -f $to_remove
> -test_expect_success \
> -    'change in branch B (removal)' \
> -    "git update-index --remove $to_remove"
> +test_expect_success 'change in branch B (removal)' '
> +    git update-index --remove $to_remove
> +'

Here also.

>  for p in ?M Z/?M
>  do
>      echo This is modified $p in the branch B. >$p
> -    test_expect_success \
> -       'change in branch B (modification)' \
> -       "git update-index $p"
> +    test_expect_success 'change in branch B (modification)' '
> +           git update-index $p
> +    '

Here also.

>  done