All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Taylor Blau" <me@ttaylorr.com>, "René Scharfe" <l.s.r@web.de>,
	"Git List" <git@vger.kernel.org>, "Jeff King" <peff@peff.net>
Subject: Re: [PATCH] p5311: handle spaces in wc(1) output
Date: Mon, 04 Oct 2021 09:16:47 -0700	[thread overview]
Message-ID: <xmqq5yucydcg.fsf@gitster.g> (raw)
In-Reply-To: 87wnmuo7ii.fsf@evledraar.gmail.com

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> This approach seems like a bit of plastering over the real problem. It's
> fine to use the output of "wc -l" or "wc -c" in the context of the
> shell's whitespace handling. That's why in various places we do:

Sorry, but I am confused.

>     test $(wc -l <$file>) = 1
>
> Or similar, but *don't* put that $() in double-quotes. I.e. we're
> relying on the shell's whitespace semantics.
>
> So isn't it better to just pass this through the shell's own handling
> before emitting the data, something like this POC:
>
>     $ stripspace() { var=$1; echo $@; }; x=$(stripspace "  hi" "  there "); echo "\"$x\""
>     "hi there"

All of the above are not wrong per-se, but if I read the scaffolding
code correctly, the way the output from "wc -c" is used is not via a
variable, but

    test_size_ () {
            say >&3 "running: $2"
            if test_eval_ "$2" 3>"$base".result; then
                    test_ok_ "$1"
            else
                    test_failure_ "$@"
            fi
    }

    test_size () {
            test_wrapper_ test_size_ "$@"
    }

where "$2" gets the script given to test_size, e.g.

 	test_size "size   $title" '
		wc -c <tmp.pack
 	'

the "wc -c" command.  And we just let the command emit its output to
"$base.result" (test_eval_ does the stdout-to-#3 redirection, and we
redirect #3 back to the file here).  So I am not quite sure where in
the current system your suggestion to apply the "substitition will
lose $IFS around values and gets word splitted if you omit dq around
it" would fit to address the issue at hand.

> Of course fixing it up after that in Perl will work just as well, so I
> guess this is just an asthetic preference for having the shell handle
> the shell's output issues with what's guaranteed to be shell-portable
> solutions... :)

Meaning we could rewrite aggregation in shell, then we can say we
are not making Perl clean up after mess sh creates?  I dunno...

Thanks.

  reply	other threads:[~2021-10-04 16:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-02 20:33 [PATCH] p5311: handle spaces in wc(1) output René Scharfe
2021-10-03  5:14 ` Taylor Blau
2021-10-03  8:04   ` Ævar Arnfjörð Bjarmason
2021-10-04 16:16     ` Junio C Hamano [this message]
2021-10-04  7:43   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq5yucydcg.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.