From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 30 Nov 2016 11:07:51 GMT
Subject: Re: [PATCH] difftool.c: mark a file-local symbol with static
Message-ID: <alpine.DEB.2.20.1611301204020.117539@virtualbox>
In-Reply-To: <4ddad7ea-5ac8-20b2-da9e-5843c486878a@ramsayjones.plus.com>

Hi Ramsay,

On Tue, 29 Nov 2016, Ramsay Jones wrote:

> If you need to re-roll your 'js/difftool-builtin' branch, could
> you please squash this into the relevant patch.

Fixed. Thanks!

> Also, due to a problem in my config.mak file on Linux (a commented
> out line that had a line continuation '\', grrrrr!), gcc issued a
> warning, thus:
> 
>   builtin/difftool.c: In function ‘run_dir_diff’:
>   builtin/difftool.c:568:13: warning: zero-length gnu_printf format string [-Wformat-zero-length]
>        warning("");
>                ^
> I am not sure why -Wno-format-zero-length is set in DEVELOPER_CFLAGS,
> but do you really need to space the output with an an 'empty'
> "warning:" line? (Just curious).

That `warning("");` comes from a straight-forward port of this line (see
https://github.com/git/git/blob/v2.11.0/git-difftool.perl#L425):

	$errmsg .= "warning:\n";

I could see two possible ways out:

- warning("%s", ""); (ugly!)

- do away with the "prefix every line with warning:" convention and simply
  have a multi-line `warning(_("...\n...\n"), ...)`

What do you think?
Dscho