From: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
Date: Thu, 28 Jun 2018 16:17:58 GMT
Subject: Re: [PATCH v6 3/4] stash: convert branch to builtin
Message-ID: <5677e7cd-a0f9-194c-a7bd-5eea48c56e3d@gmail.com>
In-Reply-To: <xmqqfu18f4zz.fsf@gitster-ct.c.googlers.com>

Hello,

On 27.06.2018 21:39, Junio C Hamano wrote:
> This is primarily because cmd_$foo() is designed to be replacement
> of "main()" in usual programs---it is allowed to assume the global
> variables it uses have their initial values and nobody cares the
> state it leaves behind when it returns.  Argument parser may flip
> bits in file-scope static variables to record which options are
> seen, assuming that these variables are initialized to all-zero, and
> that assumption would not hold for the second call to cmd_$foo(),
> etc.  cmd_$foo() also is allowed to call die() when there is an
> unrecoverable error condition in the context of carrying out $foo; a
> scripted Porcelain that used $foo as a building block to implement a
> larger operation (e.g. "stash" that used "checkout" to switch to a
> different branch) may want to react to the failure and do something
> extra (i.e. "git checkout || do something more").
> 
> Using run_command() interface would not cause any of these problems;
> the subcommand will run in a clean environment, and the caller can
> react to its failure.
> 

Thank you a lot for this great explanation. When I will submit a new 
iteration, I will make sure to replace calls of `cmd_$foo()` with 
`run_command()` (or with API, if possible).

Best regards,
Paul Ungureanu