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

Hello Eric,

Thanks for responding so quickly.  Please, see my comments below.

On 2024-03-17 05:03, Eric Sunshine wrote:

On Sat, Mar 16, 2024 at 11:48 PM Dragan Simic [off-list ref] 
wrote:

quoted

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 <redacted>
---

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
        ...
    '

Agreed, I'll take this approach in the v3.

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.

As far as I can see after doing a few really quick greps in the "t"
subdirectory, such an approach might actually make sense, but it would
require further work, to make some other already existing tests use
the enhanced q_to_tab() function, and to warrant the whole thing.

That might be an interesting #leftover for someone else to pick it up
at some point.