Re: [PATCH v3 14/25] setup.c: convert is_git_directory() to use strbuf
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 20, 2014, 19:36 UTC
- Message-ID
- <xmqqob2160fa.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <xmqqwqgp61tl.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 27 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes: > >> On Thu, Feb 20, 2014 at 3:17 AM, Junio C Hamano <gitster@pobox.com> wrote: >>>> + strbuf_setlen(sb, len); >>>> + strbuf_add(sb, s, strlen(s)); >>> >>> I am not sure addstr_at() gives us a good abstraction, or at least >>> the name conveys what it does well not to confuse readers. >>> >>> At first after only seeing its name, I would have expected that it >>> would splice the given string into an existing strbuf at the >>> location, not chopping the existing strbuf at the location and >>> appending. >> >> I think I invented a few new strbuf_* in this series and this is one >> of them. We have about ~14 other places in current code that do >> similar pattern: set length back, then add something on top. > > Yes, and you can count getline/getwholeline as a special case to > chomp to empty at the beginning. I am not opposed to a helper to > give us an easy access to this common pattern. > > It was just the name "addstr-at" did not sound, at least to me, what > it does, i.e. "replace with s from the pos to the end", which I > think is the same thing as a single-liner: > > strbuf_splice(sb, pos, sb->len - pos, s, strlen(s))
Oh, and as to other strbuf_* helpers, I am finding myself getting very fond of strbuf_git_path() as I read the series along; it gives us the same convenience as git_path() [*1*] while giving us tighter control on the lifetime rules of the path buffer.
[Footnote]
*1* And the new git_path() updated in this series has to be a lot more than catenate($GIT_DIR, "/", $path) but needs the smart of adjust_git_path(), the convenience matters.