Re: [PATCH 3/3] introduce format date-mode

2015-07-20 Thread Eric Sunshine
On Tue, Jun 30, 2015 at 9:26 AM, Jeff King p...@peff.net wrote: On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote: Clients of strbuf rightly expect the buffer to grow as needed in order to complete the requested operation. It is, therefore, both weird and expectation-breaking for

Re: [PATCH 3/3] introduce format date-mode

2015-07-20 Thread Jeff King
On Mon, Jul 20, 2015 at 08:41:08PM -0400, Eric Sunshine wrote: Here's a patch, on top of jk/date-mode-format (I think it would also be fine to just squash into the tip commit; the explanation in the commit message is sufficiently mirrored in the code comment). While cleaning up old local

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Jeff King
On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote: Clients of strbuf rightly expect the buffer to grow as needed in order to complete the requested operation. It is, therefore, both weird and expectation-breaking for strbuf_addftime() to lack this behavior. Worse, it doesn't even

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Jeff King
On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote: Clients of strbuf rightly expect the buffer to grow as needed in order to complete the requested operation. It is, therefore, both weird and expectation-breaking for strbuf_addftime() to lack this behavior. Worse, it doesn't even

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: strbuf_addf(f, %s , fmt); Basically I was trying to avoid making any assumptions about exactly how strftime works. But presumably stick a space in the format is a universally reasonable thing to do. It's a hack, but it's contained to the function. Why

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Eric Sunshine
On Tue, Jun 30, 2015 at 1:58 PM, Jeff King p...@peff.net wrote: On Tue, Jun 30, 2015 at 12:58:33PM -0400, Eric Sunshine wrote: Beyond the extra allocation, I was also concerned about the sledgehammer approach of %s to append a single character when there are much less expensive ways to do so.

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Jeff King
On Tue, Jun 30, 2015 at 09:22:18AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: strbuf_addf(f, %s , fmt); Basically I was trying to avoid making any assumptions about exactly how strftime works. But presumably stick a space in the format is a universally reasonable

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Eric Sunshine
On Tue, Jun 30, 2015 at 6:20 AM, Jeff King p...@peff.net wrote: On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote: void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm) { size_t len; struct strbuf f = STRBUF_INIT; /* * This is a

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Junio C Hamano
Jeff King p...@peff.net writes: This does get called a lot (e.g., once per commit). One extra allocation would probably not kill us there, but I think we could fairly trivially put this on the unlikely path: size_t hint = 128; size_t len; /* optimize out obvious 0-length case */

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Eric Sunshine
On Tue, Jun 30, 2015 at 9:26 AM, Jeff King p...@peff.net wrote: On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote: Clients of strbuf rightly expect the buffer to grow as needed in order to complete the requested operation. It is, therefore, both weird and expectation-breaking for

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Jeff King
On Tue, Jun 30, 2015 at 12:58:33PM -0400, Eric Sunshine wrote: Basically I was trying to avoid making any assumptions about exactly how strftime works. But presumably stick a space in the format is a universally reasonable thing to do. It's a hack, but it's contained to the function. I

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Eric Sunshine
On Tue, Jun 30, 2015 at 1:05 PM, Junio C Hamano gits...@pobox.com wrote: Answering myself to my earlier question, the reason is because I was worried what happens when given fmt is a malformed strftime format specifier. Perhaps it ends with a lone % and % may format to something unexpected,

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Jeff King
On Tue, Jun 30, 2015 at 10:05:33AM -0700, Junio C Hamano wrote: I'd guess most cases will fit in 128 bytes and never even hit this code path. You could also get fancier and start the buffer smaller, but only do the fmt hack when we cross a threshold. I'd assume that the hint thing will

Re: [PATCH 3/3] introduce format date-mode

2015-06-30 Thread Jeff King
On Tue, Jun 30, 2015 at 02:13:53PM -0400, Eric Sunshine wrote: Sorry, I meant that the interpolation expense of %s . A cheaper (but more verbose) alternative might be: size_t n = strlen(fmt); const char *f = xmalloc(n + 2); strcpy(f, fmt); f[n] = ' '; f[n + 1] = '\0';

Re: [PATCH 3/3] introduce format date-mode

2015-06-29 Thread Eric Sunshine
On Thu, Jun 25, 2015 at 12:55:45PM -0400, Jeff King wrote: This feeds the format directly to strftime. Besides being a little more flexible, the main advantage is that your system strftime may know more about your locale's preferred format (e.g., how to spell the days of the week).

[PATCH 3/3] introduce format date-mode

2015-06-25 Thread Jeff King
This feeds the format directly to strftime. Besides being a little more flexible, the main advantage is that your system strftime may know more about your locale's preferred format (e.g., how to spell the days of the week). Signed-off-by: Jeff King p...@peff.net ---