Re: [PATCH v3 2/2] advice: extract vadvise() from advise()
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Feb 20, 2020, 01:42 UTC
- Message-ID
- <20200220014205.GB2447@google.com>
- In-Reply-To
- <a2a145c705e2751d4ced9cc71e62d5c560adb6e6.1582144442.git.gitgitgadget@gmail.com>
On Wed, Feb 19, 2020 at 08:34:01PM +0000, Heba Waly via GitGitGadget wrote:
Show 5 quoted lines
> From: Heba Waly <heba.waly@gmail.com> > > 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).
Show 13 quoted lines
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> Signed-off-by: Heba Waly <heba.waly@gmail.com>
> ---
> - 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