On Wed, Feb 02 2022, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@xxxxxxxxx>
>
> This modifies the new display_update_messages() function to allow
> printing to somewhere other than stdout. It also consolidates the
> location of the diff_warn_rename_limit() message with the rest of the
> CONFLICT and other update messages to all go to the same stream.
>
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> merge-ort.c | 9 +++++----
> merge-ort.h | 3 ++-
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 82d2faf5bf9..d28d1721d14 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,13 +4264,13 @@ 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);
> + strbuf_write(sb, stream);
> }
> string_list_clear(&olist, 0);
>
> /* Also include needed rename limit adjustment now */
> diff_warn_rename_limit("merge.renamelimit",
> - opti->renames.needed_limit, 0, stderr);
> + opti->renames.needed_limit, 0, stream);
At the tip of this series I tried to s/stream/stderr/g this, and
t4301-merge-tree-write-tree.sh passes, doesn't this warning_fp() special
behavior need a test somewhere?
I assumed that warning_fp() would be using vreportf() in usage.c, but
it's not, it's just falling back to the equivalent of fprintf(out, ...),
no? I don't really see why 05/15 and parts of 06/15 & this are needed
over a much simpler simple helper macro like the below. applied on top
of this series.
I would get it if the point was to actually use the full usage.c
machinery, but we're just either calling warning(), or printing a
formatted string to a file FILE *. There's no need to go through usage.c
for that, and adding an API to it that behaves like this new
warning_fp() is really confusing.
I.e. an API in usage.c that allowed warning to a given FD would be
trying to replace the "2" in the write_in_full() call in vreportf(), I
would think.
diff --git a/diff.c b/diff.c
index 28368110147..4cf67e93dea 100644
--- a/diff.c
+++ b/diff.c
@@ -6377,14 +6377,21 @@ static const char rename_limit_advice[] =
N_("you may want to set your %s variable to at least "
"%d and retry the command.");
+#define warning_fp(out, fmt, ...) do { \
+ if (out == stderr) \
+ warning(fmt, __VA_ARGS__); \
+ else \
+ fprintf(out, fmt, __VA_ARGS__); \
+} while (0)
+
void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc,
FILE *out)
{
fflush(stdout);
if (degraded_cc)
- warning_fp(out, _(degrade_cc_to_c_warning));
+ warning_fp(out, _(degrade_cc_to_c_warning), NULL);
else if (needed)
- warning_fp(out, _(rename_limit_warning));
+ warning_fp(out, _(rename_limit_warning), NULL);
else
return;
diff --git a/git-compat-util.h b/git-compat-util.h
index 64ba60e5c71..d70ce142861 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -475,7 +475,6 @@ int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
-void warning_fp(FILE *out, const char *warn, ...) __attribute__((format (printf, 2, 3)));
#ifndef NO_OPENSSL
#ifdef APPLE_COMMON_CRYPTO
diff --git a/usage.c b/usage.c
index 0bfd2c603c0..c7d233b0de9 100644
--- a/usage.c
+++ b/usage.c
@@ -253,20 +253,6 @@ void warning(const char *warn, ...)
va_end(params);
}
-void warning_fp(FILE *out, const char *warn, ...)
-{
- va_list params;
-
- va_start(params, warn);
- if (out == stderr)
- warn_routine(warn, params);
- else {
- vfprintf(out, warn, params);
- fputc('\n', out);
- }
- va_end(params);
-}
-
/* Only set this, ever, from t/helper/, when verifying that bugs are caught. */
int BUG_exit_code;
