Subject: Re: [PATCH v8 1/6] run-command: add duplicate_output_fn to run_processes_parallel_opts

From: Junio C Hamano <[email-masked]>
Date: Mon, 13 Feb 2023 09:52:12 -0800

Glen Choo <[email-masked]> writes:

> What do we think of the name "duplicate_output"? IMO it made sense in
> earlier versions when we were copying the output to a separate buffer (I
> believe it was renamed in response to [1]), but now that we're just
> calling a callback on the main buffer, it seems misleading. Maybe
> "output_buffered" would be better?

Yeah, we do not even know what the callback does to the data we are
giving it.  The only thing we know is that we have output from the
child, and in addition to the usual buffering we do ourselves, we
are allowing the callback to peek into the buffered data in advance.

If the callback does consume it *and* remove the buffered data it
consumed right away, then as you say, "duplicate" becomes a word
that totally misses the point.  There is no duplication, as the
callback consumed and we no longer has our own copy, either.

If the callback consumes it but leaves the buffered data as-is, and
we would show that once the child finishes anyway, you can say that
we are feeding a duplicate of buffered data to the callback.  The
mechanism could be used merely to count how much output we have
accumulated so far to update the progress-bar, for example, and the
output may be given after the process is done.  But note that we are
not doing an "output" of "buffered" data in such a case.

To me, both "duplicate_output" and "output_buffered" sound like they
are names that are quite specific to the expected use case the
person who proposed the names had in mind, yet it is a bit hard to
guess exactly what the expected use cases they had in mind were,
because the names are not quite specific enough.

> Sidenote: One convention from JS that I like is to name such event
> listeners as "on_<event_name>", e.g. "on_output_buffered".

Thanks for bringing this up.  I agree that "Upon X happening, do
this" is a very good convention to follow.  I think the callback is
made whenever the child emits to the standard error stream, so
"on_error_output" (if we are worried that "error" has a too strong
"something bad happend" connotation, then perhaps "on_stderr_output"
may dampen it) perhaps?