git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Cc: git@vger.kernel.org, liu.denton@gmail.com
Subject: Re: [PATCH] builtin/merge: avoid -Wformat-extra-args from ancient Xcode
Date: Mon, 9 Aug 2021 14:13:15 -0400	[thread overview]
Message-ID: <YRFwO6TTpGYVbdNz@coredump.intra.peff.net> (raw)
In-Reply-To: <20210808033834.74470-1-carenas@gmail.com>

On Sat, Aug 07, 2021 at 08:38:34PM -0700, Carlo Marcelo Arenas Belón wrote:

> d540b70c85 (merge: cleanup messages like commit, 2019-04-17) adds
> a way to change part of the helper text using a single call to
> strbuf_add_commented_addf but with two formats with varying number
> of parameters.
> 
> this trigger a warning in old versions of Xcode (ex 8.0), so use
> instead two independent calls with a matching number of parameters

In general, if only an old version of a compiler complains, I'd prefer
to either silence it (by loosening the -W options in config.mak.dev for
systems without clang4), or just ignore it.

That said, in this case...

> -		strbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ?
> -			scissors_editor_comment :
> -			no_scissors_editor_comment), comment_line_char);
> +		if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
> +			strbuf_commented_addf(&msg, _(scissors_editor_comment));
> +		else
> +			strbuf_commented_addf(&msg,
> +				_(no_scissors_editor_comment), comment_line_char);

I think the result is actually easier to follow, as we can see that in
the CLEANUP_SCISSORS code we do not care about comment_line_char.

Thanks for the cleanup.

-Peff

      reply	other threads:[~2021-08-09 18:13 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-08  3:38 [PATCH] builtin/merge: avoid -Wformat-extra-args from ancient Xcode Carlo Marcelo Arenas Belón
2021-08-09 18:13 ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YRFwO6TTpGYVbdNz@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=liu.denton@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).