Hi Dscho,

On Fri, May 13, 2016 at 8:32 AM, Johannes Schindelin
[off-list ref] wrote:

Hi Chris,

On Wed, 11 May 2016, Christian Couder wrote:

quoted

I consider that the apply functionality is properly libified before
these patches, and that they should be in a separate series, but
unfortunately using the libified apply in "git am" unmasks the fact that
"git am", since it was a shell script, has been silencing the apply
functionality by "futzing with file descriptors". And unfortunately this
makes some reviewers unhappy.

It is a misrepresentation to claim that this makes some reviewers unhappy.
Speaking for myself, I am very happy. Despite having had to point out
that the previous iteration of this patch series had a serious flaw.

It is also incorrect to say that the shell script had been "futzing with
the file descriptors". You see, the shell script's *own* file descriptors
had been left completely unaffected by the redirection of the spawned
process' output. That was perfectly fine a thing to do, even if it
possibly hid fatal errors. Shell scripts are simply very limiting. The
problem was introduced by v1 of this patch series, which changed *the
caller's file descriptors* back and forth simply because the called code
no longer runs in a separate process. And *that* was, and is, improper.

I think we should just agree that we disagree on what we think the v1
was doing and move on to v2.

quoted

By the way there are no tests yet for this new feature, and I am not
sure at all that "--silent" and "be_silent" are good names.

If you want to follow existing code's example, we typically call this
option "quiet".

In the documentation there is:

Documentation/git-am.txt:--quiet::
Documentation/git-am.txt:       Be quiet. Only print error messages.
Documentation/git-branch.txt:--quiet::
Documentation/git-branch.txt:   Be more quiet when creating or
deleting a branch, suppressing
Documentation/git-branch.txt-   non-error messages.
Documentation/git-checkout-index.txt:--quiet::
Documentation/git-checkout-index.txt:   be quiet if files exist or are
not in the index
Documentation/git-checkout.txt:--quiet::
Documentation/git-checkout.txt- Quiet, suppress feedback messages.
Documentation/git-clean.txt:--quiet::
Documentation/git-clean.txt:    Be quiet, only report errors, but not
the files that are
Documentation/git-clean.txt-    successfully removed.
Documentation/git-clone.txt--q::
Documentation/git-clone.txt:    Operate quietly.  Progress is not
reported to the standard
Documentation/git-clone.txt-    error stream.
ocumentation/git-commit.txt:--quiet::
Documentation/git-commit.txt-   Suppress commit summary message.
Documentation/git-fast-import.txt:--quiet::
Documentation/git-fast-import.txt-      Disable all non-fatal output,
making fast-import silent when it
...

So it looks to me that --quiet means something like "don't tell the
story of your life, but in case of problem you are allowed to
complain". In other word --quiet generally doesn't suppress error
messages from error() or die().

On the contrary the new feature I tentatively called --silent does
suppress all output including error messages from error().

Now if people think that it is not worth making a difference between
the different behaviors, then I am ok to rename it --quiet, though I
wonder what will happen if people later want a --quiet that does only
what --quiet usually does to the other commands.

quoted

Sorry if this patch series is long. I can split it into two or more
series if it is prefered.

It is preferred. Much.

Ok, I will split it then.

Thanks,
Christian.