Re: [PATCH v5 02/12] ref-filter: use strbuf_split_str_omit_term()

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v5 02/12] ref-filter: use strbuf_split_str_omit_term()

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:16

Jeff King [off-list ref] writes:
On Tue, Feb 16, 2016 at 04:12:08PM -0800, Junio C Hamano wrote:
quoted
quoted
To be honest, though, I am now on the fence, considering the possible
whitespace issue.
Certainly not having to see s[0]->buf over and over is a huge win ;-).

Is the "whitespace issue" a big deal?  Does it involve more than a
similar sibling to string_list_split() that trims the whitespace
around the delimiter (or allows a regexp as a delimiter "\s*,\s*")?
I think that solution would work (and IMHO would actually be preferable
to the split-then-trim that strbuf_split does). But it does mean writing
new code.
True, but only when we decide to support trimming the whitespace,
which can come later.

I do not even know if it is wise to accept %(align:position=left, width=4)
when %(align:position=left,width=4) would do the job just fine.

Re: [PATCH v5 02/12] ref-filter: use strbuf_split_str_omit_term()

From: Jeff King <hidden>
Date: 2016-06-15 23:08:16

On Tue, Feb 16, 2016 at 04:28:10PM -0800, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
On Tue, Feb 16, 2016 at 04:12:08PM -0800, Junio C Hamano wrote:
quoted
quoted
To be honest, though, I am now on the fence, considering the possible
whitespace issue.
Certainly not having to see s[0]->buf over and over is a huge win ;-).

Is the "whitespace issue" a big deal?  Does it involve more than a
similar sibling to string_list_split() that trims the whitespace
around the delimiter (or allows a regexp as a delimiter "\s*,\s*")?
I think that solution would work (and IMHO would actually be preferable
to the split-then-trim that strbuf_split does). But it does mean writing
new code.
True, but only when we decide to support trimming the whitespace,
which can come later.

I do not even know if it is wise to accept %(align:position=left, width=4)
when %(align:position=left,width=4) would do the job just fine.
Yeah, it was mostly just about being friendly to the user. But if nobody
is complaining, it may not even be worth worrying about.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help