On Wed, Feb 19, 2020 at 08:34:01PM +0000, Heba Waly via GitGitGadget wrote:
> From: Heba Waly <heba.waly@xxxxxxxxx>
>
> extract a version of advise() that uses an explict 'va_list' parameter.
> Call it from advise() and advise_if_enabled() for a functionally
> equivalent version.
Hm, I'd put this patch before the advise_if_enabled() one, so that each
commit makes sense by itself (rather than adding a bunch of code last
patch only to remove it in this patch).
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> Signed-off-by: Heba Waly <heba.waly@xxxxxxxxx>
> ---
> - for (cp = buf.buf; *cp; cp = np) {
> - np = strchrnul(cp, '\n');
> - fprintf(stderr, _("%shint: %.*s%s\n"),
> - advise_get_color(ADVICE_COLOR_HINT),
> - (int)(np - cp), cp,
> - advise_get_color(ADVICE_COLOR_RESET));
> - if (*np)
> - np++;
> - }
I see - this hunk that I commented on in the other review is actually
duplicated from advise(). Hm, I still think it'd be useful to put this
functionality into strbuf, but I guess since it's not new code you're
adding there's not a lot of need to sweat about it.
- Emily
