Re: [PATCH v3 2/2] strbuf: change an always NULL/"" strbuf_addftime() param to bool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jun 24, 2017 at 11:36:35AM +0000, Ævar Arnfjörð Bjarmason wrote:

> >>  extern void strbuf_addftime(struct strbuf *sb, const char *fmt,
> >>  			    const struct tm *tm, int tz_offset,
> >> -			    const char *tz_name);
> >> +			    const int omit_strftime_tz_name);
> >
> > This would need the new name, too (whatever it is).
> 
> *Nod*. Now the parameter is called suppress_tz_name.

Thanks. That sounds good (and your initial re-ordering patch looks fine,
too). One minor typo:

> diff --git a/strbuf.h b/strbuf.h
> index 6708cef0f9..d3e6e65123 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -343,11 +343,12 @@ extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
>   * `tz_offset` is in decimal hhmm format, e.g. -600 means six hours west
>   * of Greenwich, and it's used to expand %z internally.  However, tokens
>   * with modifiers (e.g. %Ez) are passed to `strftime`.
> - * `tz_name` is used to expand %Z internally unless it's NULL.
> + * `suppress_tz_name` when set, means let suppress the `strftime` %Z
> + * format and replace it with an empty string.

I couldn't quite parse "let suppress". I'm not sure if it was supposed
to be "let's". Probably "means to suppress the strftime..." would be
more clear. I'd probably have written it more like:

  `suppress_tz_name`, when set, expands %Z internally to the empty
  string rather than passing it to `strftime`.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux