Re: [PATCH 06/12] merge-ort: allow update messages to be written to different file stream
From: Johannes Schindelin <hidden>
Date: 2022-01-28 16:31:17
Subsystem:
the rest · Maintainer:
Linus Torvalds
Hi Elijah, On Sat, 22 Jan 2022, Elijah Newren via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
@@ -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