Re: [PATCH v2 3/5] test: introduce new x_to_tab() helper function

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 17, 2024, 04:03 UTC
Message-ID: <CAPig+cSLb+Rsy81itvw9Tfvqv9vvKSPgO_ER9fWL04XZrgFvwg@mail.gmail.com>
In-Reply-To: <480b46f2db82ea9c6cd9bbc2423923f81f2d36f5.1710646998.git.dsimic@manjaro.org>

On Sat, Mar 16, 2024 at 11:48 PM Dragan Simic <dsimic@manjaro.org> wrote:

Show 16 quoted lines

> There's nothing wrong with the already existing q_to_tab() function, except
> when it's used on strings that contain uppercase letter "Q" in its literal
> meaning, which, for example, can happen with git configurations that contain
> "*.*Quoted" as the names of their configuration variables.
>
> Thus, let's introduce new x_to_tab() helper function that does pretty much
> the same job as the already existing q_to_tab() helper function, except for
> replacing "X" with a horizontal tab (HT), instead of replacing "Q".
>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -107,6 +107,10 @@ q_to_tab () {
> +x_to_tab () {
> +       tr X '\011'
> +}

I'd like to push back on this change since it may lead to an explosion of new almost-identical functions. For such a one-off case where q_to_tab() isn't appropriate, it's perfectly fine to simply use `tr X `\011'` directly in your test:

    test_expect_success 'foo' '
        tr X "\011" >expect <<-\EOF
        some Q stuff
        whitespaceXhere
        EOF
        ...
    '

However, if you really insist upon using a library function, then either add a general-purpose function which accepts the special character as an argument, or just retrofit q_to_tab() to optionally accept the special character:

    # t/test-lib-functions.sh

    # usage: q_to_tab [<needle-char>]
    # replace <needle-char> with TAB in stdin
    q_to_tab () {
        local c=$1
        test -n "$c" || c=Q
        tr "$c" '\011'
    }

But this is probably overkill for a one-off case.