On Fri, Jul 16, 2021 at 12:53:17PM -0400, Taylor Blau wrote:
> > packetize () {
> > if test $# -gt 0
> > then
> > packet="$*"
> > printf '%04x%s' "$((4 + ${#packet}))" "$packet"
> > else
> > - perl -e '
> > - my $packet = do { local $/; <STDIN> };
> > - printf "%04x%s", 4 + length($packet), $packet;
> > - '
> > + test-tool pkt-line pack
> > fi
> > }
>
> For what it's worth, I would be happy to remove the printf shortcut
> entirely. Some quick grepping indicates only 22 uses of the word
> "packetize" in our whole test suite (one of them being the function
> declaration itself). And of the 21 callers, only 10 pass at least one
> argument:
>
> git grep -Ew 'packetize [^()&]+' -- t
>
> So I would be fine with adding 10 more new processes to the test suite
> in the name of simplifying this declaration. I don't feel strongly,
> though, since the conditional here does not really add that much
> complexity.
I'd be fine with that, too. Part of the goal of the cmdline option was
making callers more readable. E.g.,
-printf "want %s" "$hash_head" | packetize
+packetize "want $hash_head"
But I think the here-doc input to "test-tool pkt-line" is generally
equally nice, especially when you have multiple lines.
(the commit introducing the cmdline version talks about efficiency, but
there the old version was using 4 processes!)
-Peff
