Re: [PATCH 10/11] run command: add RUN_COMMAND_NO_STDOUT

Kevin Wern [off-list ref] writes:

Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child
process.

This will be used by git clone when calling index-pack on a downloaded
packfile.

If it is just one caller, would't it make more sense for that caller
set no_stdout explicitly itself?

Hi,

On Fri, 16 Sep 2016, Junio C Hamano wrote:

Kevin Wern [off-list ref] writes:

quoted

Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child
process.

This will be used by git clone when calling index-pack on a downloaded
packfile.

If it is just one caller, would't it make more sense for that caller
set no_stdout explicitly itself?

Taking a step back, maybe it is not such a good idea to swallow the output
in all cases, including the error cases?

Maybe the best course of action is to hide stdout/stderr by default but
show it in case of a non-zero exit code, i.e. using
https://public-inbox.org/git/6383b7afcdeb6c999862aa32ba437997f2dd3d4e.1472633606.git.johannes.schindelin@gmx.de/ ?

Ciao,
Dscho

On Fri, Sep 16, 2016 at 04:07:00PM -0700, Junio C Hamano wrote:

Kevin Wern [off-list ref] writes:

quoted

Add option RUN_COMMAND_NO_STDOUT, which sets no_stdout on a child
process.

This will be used by git clone when calling index-pack on a downloaded
packfile.

If it is just one caller, would't it make more sense for that caller
set no_stdout explicitly itself?

I based the calling code in do_index_pack on dissociate_from_references, which
uses run_command_v_opt, so it never occured to me to do that. I thought it was
just good, uniform style and encapsulation. Like how transport's methods and
internals aren't really intended to be changed or accessed--unless it's through
the APIs we create.

However, I don't feel very strongly about this, so I'm okay with this change.