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