Hi Elijah,
On Sat, 22 Jan 2022, Elijah Newren via GitGitGadget wrote:
> diff --git a/merge-ort.c b/merge-ort.c
> index f9e35b0f96b..b78dde55ad9 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -4236,7 +4236,8 @@ static int record_conflicted_index_entries(struct merge_options *opt)
> }
>
> void merge_display_update_messages(struct merge_options *opt,
> - struct merge_result *result)
> + struct merge_result *result,
> + FILE *stream)
> {
> struct merge_options_internal *opti = result->priv;
> struct hashmap_iter iter;
> @@ -4263,7 +4264,7 @@ void merge_display_update_messages(struct merge_options *opt,
> for (i = 0; i < olist.nr; ++i) {
> struct strbuf *sb = olist.items[i].util;
>
> - printf("%s", sb->buf);
> + fprintf(stream, "%s", sb->buf);
Maybe `strbuf_write(sb, stream);` instead? Whenever I see a `"%s"`, I feel
like it's unnecessary churn...
> }
> string_list_clear(&olist, 0);
>
Missing from this hunk:
/* Also include needed rename limit adjustment now */
diff_warn_rename_limit("merge.renamelimit",
opti->renames.needed_limit, 0);
This function explicitly writes to `stdout`, and will need to be adjusted,
I think, before we can include an adjustment to this call in this patch.
Unless we override `warn_routine()` (which is used inside that function),
that is. Which is hacky, and we would not have addressed the
`fflush(stdout)` in `diff_warn_rename_limit()`. So I would much prefer
something like this:
-- snip --
diff --git a/diff.c b/diff.c
index 861282db1c3..87966602cbd 100644
--- a/diff.c
+++ b/diff.c
@@ -6312,17 +6312,25 @@ static const char rename_limit_advice[] =
N_("you may want to set your %s variable to at least "
"%d and retry the command.");
-void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
+void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc,
+ FILE *out)
{
- fflush(stdout);
+ const char *fmt = NULL;
+
if (degraded_cc)
- warning(_(degrade_cc_to_c_warning));
+ fmt = _(degrade_cc_to_c_warning);
else if (needed)
- warning(_(rename_limit_warning));
+ fmt = _(rename_limit_warning);
else
return;
if (0 < needed)
- warning(_(rename_limit_advice), varname, needed);
+ fmt = _(rename_limit_advice);
+
+ fflush(out);
+ if (out == stdout)
+ warning(fmt, varname, needed);
+ else
+ fprintf(out, fmt, varname, needed);
}
static void diff_flush_patch_all_file_pairs(struct diff_options *o)
@@ -6754,7 +6762,7 @@ int diff_result_code(struct diff_options *opt, int status)
diff_warn_rename_limit("diff.renameLimit",
opt->needed_rename_limit,
- opt->degraded_cc_to_c);
+ opt->degraded_cc_to_c, stdout);
if (!opt->flags.exit_with_status &&
!(opt->output_format & DIFF_FORMAT_CHECKDIFF))
return status;
diff --git a/diff.h b/diff.h
index 8ba85c5e605..be4ee68c0a2 100644
--- a/diff.h
+++ b/diff.h
@@ -596,7 +596,8 @@ void diffcore_fix_diff_index(void);
int diff_queue_is_empty(void);
void diff_flush(struct diff_options*);
void diff_free(struct diff_options*);
-void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc);
+void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc,
+ FILE *out);
/* diff-raw status letters */
#define DIFF_STATUS_ADDED 'A'
diff --git a/merge-ort.c b/merge-ort.c
index 0342f104836..e6b5a0e7c64 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4264,7 +4264,7 @@ void merge_switch_to_result(struct merge_options *opt,
/* Also include needed rename limit adjustment now */
diff_warn_rename_limit("merge.renamelimit",
- opti->renames.needed_limit, 0);
+ opti->renames.needed_limit, 0, stdout);
trace2_region_leave("merge", "display messages", opt->repo);
}
diff --git a/merge-recursive.c b/merge-recursive.c
index d9457797dbb..10b2948678c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3731,7 +3731,8 @@ static void merge_finalize(struct merge_options *opt)
strbuf_release(&opt->obuf);
if (show(opt, 2))
diff_warn_rename_limit("merge.renamelimit",
- opt->priv->needed_rename_limit, 0);
+ opt->priv->needed_rename_limit, 0,
+ stdout);
FREE_AND_NULL(opt->priv);
}
-- snap --
The rest of the patch looks good to me.
Thanks,
Dscho
> @@ -4313,7 +4314,7 @@ void merge_switch_to_result(struct merge_options *opt,
> }
>
> if (display_update_msgs)
> - merge_display_update_messages(opt, result);
> + merge_display_update_messages(opt, result, stdout);
>
> merge_finalize(opt, result);
> }
> diff --git a/merge-ort.h b/merge-ort.h
> index e5aec45b18f..d643b47cb7c 100644
> --- a/merge-ort.h
> +++ b/merge-ort.h
> @@ -86,7 +86,8 @@ void merge_switch_to_result(struct merge_options *opt,
> * so only call this when bypassing merge_switch_to_result().
> */
> void merge_display_update_messages(struct merge_options *opt,
> - struct merge_result *result);
> + struct merge_result *result,
> + FILE *stream);
>
> /* Do needed cleanup when not calling merge_switch_to_result() */
> void merge_finalize(struct merge_options *opt,
> --
> gitgitgadget
>
>
