Re: [PATCH v8 34/41] apply: add 'be_silent' variable to 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> This variable should prevent anything to be printed on both stderr
> and stdout.

It is far more important to describe "why" this is needed than what
it does, the latter of which can be read from the patch text.

And I do not see any "why" here.  Is this "when the current caller
wanted to silence us, it spawned us in a separate process and
redirected our output to /dev/null, but we no longer can do that
because we will change the calling convention to allow direct calls
into us"?

Do we have a precedent to name a switch that we usually call "quiet"
or "silent" as "be_{silent,quiet}"?  Is there already a "silent"
nearby that records what the end-user gave us (e.g. via "--silent"
option), a new name may be needed, but if that is the motivation,
I'd probably call it something more specific, "apply_silently" or
somesuch.

> Let's not take care of stdout and apply_verbosely for now though,
> as that will be taken care of in following patches.
>
> Signed-off-by: Christian Couder 
> ---
>  apply.c | 43 +--
>  apply.h |  1 +
>  2 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/apply.c b/apply.c
> index 7bf12a7..802fa79 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -1617,8 +1617,9 @@ static void record_ws_error(struct apply_state *state,
>   return;
>  
>   err = whitespace_error_string(result);
> - fprintf(stderr, "%s:%d: %s.\n%.*s\n",
> - state->patch_input_file, linenr, err, len, line);
> + if (!state->be_silent)
> + fprintf(stderr, "%s:%d: %s.\n%.*s\n",
> + state->patch_input_file, linenr, err, len, line);
>   free(err);
>  }
>  
> @@ -1813,7 +1814,7 @@ static int parse_single_patch(struct apply_state *state,
>   return error(_("new file %s depends on old contents"), 
> patch->new_name);
>   if (0 < patch->is_delete && newlines)
>   return error(_("deleted file %s still has contents"), 
> patch->old_name);
> - if (!patch->is_delete && !newlines && context)
> + if (!patch->is_delete && !newlines && context && !state->be_silent)
>   fprintf_ln(stderr,
>  _("** warning: "
>"file %s becomes empty but is not deleted"),
> @@ -3038,8 +3039,8 @@ static int apply_one_fragment(struct apply_state *state,
>* Warn if it was necessary to reduce the number
>* of context lines.
>*/
> - if ((leading != frag->leading) ||
> - (trailing != frag->trailing))
> + if ((leading != frag->leading ||
> +  trailing != frag->trailing) && !state->be_silent)
>   fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
>" to apply fragment at %d"),
>  leading, trailing, applied_pos+1);
> @@ -3536,7 +3537,8 @@ static int try_threeway(struct apply_state *state,
>read_blob_object(, pre_sha1, patch->old_mode))
>   return error("repository lacks the necessary blob to fall back 
> on 3-way merge.");
>  
> - fprintf(stderr, "Falling back to three-way merge...\n");
> + if (!state->be_silent)
> + fprintf(stderr, "Falling back to three-way merge...\n");
>  
>   img = strbuf_detach(, );
>   prepare_image(_image, img, len, 1);
> @@ -3566,7 +3568,9 @@ static int try_threeway(struct apply_state *state,
>   status = three_way_merge(image, patch->new_name,
>pre_sha1, our_sha1, post_sha1);
>   if (status < 0) {
> - fprintf(stderr, "Failed to fall back on three-way merge...\n");
> + if (!state->be_silent)
> + fprintf(stderr,
> + "Failed to fall back on three-way merge...\n");
>   return status;
>   }
>  
> @@ -3578,9 +3582,15 @@ static int try_threeway(struct apply_state *state,
>   hashcpy(patch->threeway_stage[0].hash, pre_sha1);
>   hashcpy(patch->threeway_stage[1].hash, our_sha1);
>   hashcpy(patch->threeway_stage[2].hash, post_sha1);
> - fprintf(stderr, "Applied patch to '%s' with conflicts.\n", 
> patch->new_name);
> + if (!state->be_silent)
> + fprintf(stderr,
> + "Applied patch to '%s' with conflicts.\n",
> + patch->new_name);
>   } else {
> - fprintf(stderr, "Applied patch to '%s' cleanly.\n", 
> patch->new_name);
> + if (!state->be_silent)
> + fprintf(stderr,
> + "Applied patch to '%s' cleanly.\n",
> + patch->new_name);
>   }
>   return 0;
>  }
> @@ -4483,7 +4493,8 @@ static int write_out_one_reject(struct apply_state 
> 

[PATCH v8 34/41] apply: add 'be_silent' variable to 'struct apply_state'

[Christian Couder]

This variable should prevent anything to be printed on both stderr
and stdout.

Let's not take care of stdout and apply_verbosely for now though,
as that will be taken care of in following patches.

Signed-off-by: Christian Couder 
---
 apply.c | 43 +--
 apply.h |  1 +
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/apply.c b/apply.c
index 7bf12a7..802fa79 100644
--- a/apply.c
+++ b/apply.c
@@ -1617,8 +1617,9 @@ static void record_ws_error(struct apply_state *state,
return;
 
err = whitespace_error_string(result);
-   fprintf(stderr, "%s:%d: %s.\n%.*s\n",
-   state->patch_input_file, linenr, err, len, line);
+   if (!state->be_silent)
+   fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+   state->patch_input_file, linenr, err, len, line);
free(err);
 }
 
@@ -1813,7 +1814,7 @@ static int parse_single_patch(struct apply_state *state,
return error(_("new file %s depends on old contents"), 
patch->new_name);
if (0 < patch->is_delete && newlines)
return error(_("deleted file %s still has contents"), 
patch->old_name);
-   if (!patch->is_delete && !newlines && context)
+   if (!patch->is_delete && !newlines && context && !state->be_silent)
fprintf_ln(stderr,
   _("** warning: "
 "file %s becomes empty but is not deleted"),
@@ -3038,8 +3039,8 @@ static int apply_one_fragment(struct apply_state *state,
 * Warn if it was necessary to reduce the number
 * of context lines.
 */
-   if ((leading != frag->leading) ||
-   (trailing != frag->trailing))
+   if ((leading != frag->leading ||
+trailing != frag->trailing) && !state->be_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
 " to apply fragment at %d"),
   leading, trailing, applied_pos+1);
@@ -3536,7 +3537,8 @@ static int try_threeway(struct apply_state *state,
 read_blob_object(, pre_sha1, patch->old_mode))
return error("repository lacks the necessary blob to fall back 
on 3-way merge.");
 
-   fprintf(stderr, "Falling back to three-way merge...\n");
+   if (!state->be_silent)
+   fprintf(stderr, "Falling back to three-way merge...\n");
 
img = strbuf_detach(, );
prepare_image(_image, img, len, 1);
@@ -3566,7 +3568,9 @@ static int try_threeway(struct apply_state *state,
status = three_way_merge(image, patch->new_name,
 pre_sha1, our_sha1, post_sha1);
if (status < 0) {
-   fprintf(stderr, "Failed to fall back on three-way merge...\n");
+   if (!state->be_silent)
+   fprintf(stderr,
+   "Failed to fall back on three-way merge...\n");
return status;
}
 
@@ -3578,9 +3582,15 @@ static int try_threeway(struct apply_state *state,
hashcpy(patch->threeway_stage[0].hash, pre_sha1);
hashcpy(patch->threeway_stage[1].hash, our_sha1);
hashcpy(patch->threeway_stage[2].hash, post_sha1);
-   fprintf(stderr, "Applied patch to '%s' with conflicts.\n", 
patch->new_name);
+   if (!state->be_silent)
+   fprintf(stderr,
+   "Applied patch to '%s' with conflicts.\n",
+   patch->new_name);
} else {
-   fprintf(stderr, "Applied patch to '%s' cleanly.\n", 
patch->new_name);
+   if (!state->be_silent)
+   fprintf(stderr,
+   "Applied patch to '%s' cleanly.\n",
+   patch->new_name);
}
return 0;
 }
@@ -4483,7 +4493,8 @@ static int write_out_one_reject(struct apply_state 
*state, struct patch *patch)
"Applying patch %%s with %d rejects...",
cnt),
cnt);
-   say_patch_name(stderr, sb.buf, patch);
+   if (!state->be_silent)
+   say_patch_name(stderr, sb.buf, patch);
strbuf_release();
 
cnt = strlen(patch->new_name);
@@ -4510,10 +4521,12 @@ static int write_out_one_reject(struct apply_state 
*state, struct patch *patch)
 frag;
 cnt++, frag = frag->next) {
if (!frag->rejected) {
-   fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
+   if (!state->be_silent)
+   fprintf_ln(stderr, _("Hunk #%d applied 
cleanly."), cnt);
continue;
}
-   fprintf_ln(stderr,