git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
René Scharfe <l.s.r@web.de>
Date
Jun 23, 2017, 16:22 UTC
Message-ID
<eb492fbd-5cf8-4014-acee-d9aca84dea24@web.de>
In-Reply-To
<20170623152554.jybv2vtsnjsoedoi@sigill.intra.peff.net>
Am 23.06.2017 um 17:25 schrieb Jeff King:
Show 22 quoted lines
> On Fri, Jun 23, 2017 at 05:20:19PM +0200, René Scharfe wrote:
> 
>>> diff --git a/strbuf.c b/strbuf.c
>>> index be3b9e37b1..81ff3570e2 100644
>>> --- a/strbuf.c
>>> +++ b/strbuf.c
>>> @@ -786,7 +786,7 @@ char *xstrfmt(const char *fmt, ...)
>>>    }
>>>    void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,
>>> -		     int tz_offset, const char *tz_name)
>>> +		     int tz_offset, const int omit_strftime_tz_name)
>>
>> Why const?  And as written above, naming the parameter local would make
>> it easier to understand instead of exposing an implementation detail in
>> the interface.
> 
> I think calling it "local" isn't right. That's a decision the _caller_
> is making about whether to pass through %Z. But the actual
> implementation is more like "should the function fill tzname based on
> tz?" So some name along those lines would make sense.
> 
> In which case the caller would then pass "!mode->local" for the flag.

We only have a single caller currently, so responsibilities can still be shifted, and it's a bit hard to draw the line. "Here's a format and all time information I have, expand!" is just as viable as "here's a format and most time information, expand, and handle %Z in this particular way when you see it!", I think.

René
Previous: Jeff King
Message 29 of 29 in “strbuf: change an always NULL/"" strbuf_addftime() param to bool”
  1. strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jun 23, 2017
  2. Jeff KingJun 23, 2017
  3. Ævar Arnfjörð BjarmasonJun 23, 2017
  4. Jeff KingJun 23, 2017
  5. René ScharfeJun 23, 2017
  6. Jeff KingJun 23, 2017
  7. Ævar Arnfjörð BjarmasonJun 24, 2017
  8. strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jun 23, 2017
  9. Jeff KingJun 23, 2017
  10. 1/2 strbuf.h comment: discuss strbuf_addftime() arguments in orderÆvar Arnfjörð Bjarmason, Jun 24, 2017
  11. 2/2 strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jun 24, 2017
  12. Jeff KingJun 24, 2017
  13. 1/2 strbuf.h comment: discuss strbuf_addftime() arguments in orderÆvar Arnfjörð Bjarmason, Jun 24, 2017
  14. 2/2 strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jun 24, 2017
  15. Jeff KingJun 24, 2017
  16. 1/2 strbuf.h comment: discuss strbuf_addftime() arguments in orderÆvar Arnfjörð Bjarmason, Jun 24, 2017
  17. 2/2 strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jun 24, 2017
  18. Jeff KingJun 24, 2017
  19. René ScharfeJun 24, 2017
  20. Junio C HamanoJun 24, 2017
  21. 1/2 strbuf.h comment: discuss strbuf_addftime() arguments in orderÆvar Arnfjörð Bjarmason, Jul 1, 2017
  22. 2/2 strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jul 1, 2017
  23. René ScharfeJul 1, 2017
  24. 1/2 strbuf.h comment: discuss strbuf_addftime() arguments in orderÆvar Arnfjörð Bjarmason, Jul 1, 2017
  25. 2/2 strbuf: change an always NULL/"" strbuf_addftime() param to boolÆvar Arnfjörð Bjarmason, Jul 1, 2017
  26. Junio C HamanoJun 23, 2017
  27. René ScharfeJun 23, 2017
  28. Jeff KingJun 23, 2017
  29. René ScharfeJun 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.