On Fri, Mar 15, 2024 at 9:22 AM Dragan Simic <dsimic@xxxxxxxxxxx> wrote:
> Add a handful of additional automated tests, to improve the coverage of
> configuration file entries whose values contain internal whitespace, leading
> and/or trailing whitespace, or which contain an additional inline comment.
>
> Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>
Just some minor style-related comments...
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> @@ -11,6 +11,96 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +cat > .git/config << EOF
> +[section]
> + solid = rock
> + sparse = big blue
> + sparseAndTail = big blue
> + sparseAndTailQuoted = "big blue "
> + sparseAndBiggerTail = big blue
> + sparseAndBiggerTailQuoted = "big blue "
> + sparseAndBiggerTailQuotedPlus = "big blue "
> + headAndTail = big blue
> + headAndTailQuoted = " big blue "
> + headAndTailQuotedPlus = " big blue "
> + annotated = big blue # to be discarded
> + annotatedQuoted = "big blue" # to be discarded
> +EOF
These days we try to place all test-related code inside a
test_expect_success() context rather than having it standalone. In
this case, since the file being created is (presumably) shared by
multiple tests in this script, you may want to add a new test which
performs this setup step.
Use \EOF rather than EOF to signal to readers that we don't expect any
variable interpolation to happen within the here-doc body.
Further, use -\EOF inside test_expect_success() to allow us to indent
the body of the heredoc to match the test indentation.
Style guideline says to omit whitespace after redirection operators
(such as `<<` and `>`).
We have a q_to_tab() function which lets us state explicitly for
readers the location of TAB characters in the heredoc body. You'll
often see that used instead of literal TABs.
Taking all the above into account, perhaps:
test_expect_success 'setup whitespace' '
q_to_tab >.git/config <<-\EOF
[section]
solid = rock
sparse = bigQblue
...
EOF
Same comments apply to rest of patch.
