# [PATCH 0/3] ls-tree fixes 11 messages from 2014-11-30 to 2014-12-08. Participants: Nguyễn Thái Ngọc Duy, Junio C Hamano, Duy Nguyen, Eric Sunshine. Thread: https://gitlist.dev/t/38076 ## Nguyễn Thái Ngọc Duy, 2014-11-30 09:04 Subject: [PATCH 0/3] ls-tree fixes Message-ID: <1417338302-8208-1-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1417338302-8208-1-git-send-email-pclouds%40gmail.com ``` The first two fix ls-tree's unable to handle relative paths outside $PWD. The last one rejects negative pathspec. This is a resend from http://thread.gmane.org/gmane.comp.version-control.git/259233/focus=259264 Nguyễn Thái Ngọc Duy (3): tree.c: update read_tree_recursive callback to pass strbuf as base ls-tree: remove path filtering logic in show_tree ls-tree: disable negative pathspec because it's not supported archive.c | 34 +++++++++++++++++++++------------- builtin/checkout.c | 8 ++++---- builtin/log.c | 2 +- builtin/ls-tree.c | 20 +++++++++++--------- merge-recursive.c | 15 ++++++--------- quote.c | 21 --------------------- quote.h | 2 -- t/t3102-ls-tree-wildcards.sh | 8 ++++++++ tree.c | 16 +++++++++++----- tree.h | 3 ++- 10 files changed, 64 insertions(+), 65 deletions(-) -- 2.2.0.60.gb7b3c64 ``` ## Nguyễn Thái Ngọc Duy, 2014-11-30 09:05 Subject: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base Message-ID: <1417338302-8208-2-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1417338302-8208-2-git-send-email-pclouds%40gmail.com In-Reply-To: <1417338302-8208-1-git-send-email-pclouds@gmail.com> ``` This allows the callback to use 'base' as a temporary buffer to quickly assemble full path "without" extra allocation. The callback has to restore it afterwards of course. Helped-by: Eric Sunshine Signed-off-by: Nguyễn Thái Ngọc Duy --- archive.c | 34 +++++++++++++++++++++------------- builtin/checkout.c | 8 ++++---- builtin/log.c | 2 +- builtin/ls-tree.c | 9 +++++---- merge-recursive.c | 15 ++++++--------- tree.c | 16 +++++++++++----- tree.h | 3 ++- 7 files changed, 50 insertions(+), 37 deletions(-) diff --git a/archive.c b/archive.c index 94a9981..9e30246 100644 --- a/archive.c +++ b/archive.c @@ -157,18 +157,26 @@ static int write_archive_entry(const unsigned char *sha1, const char *base, return write_entry(args, sha1, path.buf, path.len, mode); } +static int write_archive_entry_buf(const unsigned char *sha1, struct strbuf *base, + const char *filename, unsigned mode, int stage, + void *context) +{ + return write_archive_entry(sha1, base->buf, base->len, + filename, mode, stage, context); +} + static void queue_directory(const unsigned char *sha1, - const char *base, int baselen, const char *filename, + struct strbuf *base, const char *filename, unsigned mode, int stage, struct archiver_context *c) { struct directory *d; - d = xmallocz(sizeof(*d) + baselen + 1 + strlen(filename)); + d = xmallocz(sizeof(*d) + base->len + 1 + strlen(filename)); d->up = c->bottom; - d->baselen = baselen; + d->baselen = base->len; d->mode = mode; d->stage = stage; c->bottom = d; - d->len = sprintf(d->path, "%.*s%s/", baselen, base, filename); + d->len = sprintf(d->path, "%.*s%s/", (int)base->len, base->buf, filename); hashcpy(d->sha1, sha1); } @@ -191,28 +199,28 @@ static int write_directory(struct archiver_context *c) } static int queue_or_write_archive_entry(const unsigned char *sha1, - const char *base, int baselen, const char *filename, + struct strbuf *base, const char *filename, unsigned mode, int stage, void *context) { struct archiver_context *c = context; while (c->bottom && - !(baselen >= c->bottom->len && - !strncmp(base, c->bottom->path, c->bottom->len))) { + !(base->len >= c->bottom->len && + !strncmp(base->buf, c->bottom->path, c->bottom->len))) { struct directory *next = c->bottom->up; free(c->bottom); c->bottom = next; } if (S_ISDIR(mode)) { - queue_directory(sha1, base, baselen, filename, + queue_directory(sha1, base, filename, mode, stage, c); return READ_TREE_RECURSIVE; } if (write_directory(c)) return -1; - return write_archive_entry(sha1, base, baselen, filename, mode, + return write_archive_entry(sha1, base->buf, base->len, filename, mode, stage, context); } @@ -260,7 +268,7 @@ int write_archive_entries(struct archiver_args *args, err = read_tree_recursive(args->tree, "", 0, 0, &args->pathspec, args->pathspec.has_wildcard ? queue_or_write_archive_entry : - write_archive_entry, + write_archive_entry_buf, &context); if (err == READ_TREE_RECURSIVE) err = 0; @@ -286,14 +294,14 @@ static const struct archiver *lookup_archiver(const char *name) return NULL; } -static int reject_entry(const unsigned char *sha1, const char *base, - int baselen, const char *filename, unsigned mode, +static int reject_entry(const unsigned char *sha1, struct strbuf *base, + const char *filename, unsigned mode, int stage, void *context) { int ret = -1; if (S_ISDIR(mode)) { struct strbuf sb = STRBUF_INIT; - strbuf_addstr(&sb, base); + strbuf_addbuf(&sb, base); strbuf_addstr(&sb, filename); if (!match_pathspec(context, sb.buf, sb.len, 0, NULL, 1)) ret = READ_TREE_RECURSIVE; diff --git a/builtin/checkout.c b/builtin/checkout.c index 5410dac..8adf48d 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -62,7 +62,7 @@ static int post_checkout_hook(struct commit *old, struct commit *new, } -static int update_some(const unsigned char *sha1, const char *base, int baselen, +static int update_some(const unsigned char *sha1, struct strbuf *base, const char *pathname, unsigned mode, int stage, void *context) { int len; @@ -71,11 +71,11 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen, if (S_ISDIR(mode)) return READ_TREE_RECURSIVE; - len = baselen + strlen(pathname); + len = base->len + strlen(pathname); ce = xcalloc(1, cache_entry_size(len)); hashcpy(ce->sha1, sha1); - memcpy(ce->name, base, baselen); - memcpy(ce->name + baselen, pathname, len - baselen); + memcpy(ce->name, base->buf, base->len); + memcpy(ce->name + base->len, pathname, len - base->len); ce->ce_flags = create_ce_flags(0) | CE_UPDATE; ce->ce_namelen = len; ce->ce_mode = create_ce_mode(mode); diff --git a/builtin/log.c b/builtin/log.c index 734aab3..f2a9f01 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -489,7 +489,7 @@ static int show_tag_object(const unsigned char *sha1, struct rev_info *rev) } static int show_tree_object(const unsigned char *sha1, - const char *base, int baselen, + struct strbuf *base, const char *pathname, unsigned mode, int stage, void *context) { printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : ""); diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 51184df..1ab0381 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -61,7 +61,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname) } } -static int show_tree(const unsigned char *sha1, const char *base, int baselen, +static int show_tree(const unsigned char *sha1, struct strbuf *base, const char *pathname, unsigned mode, int stage, void *context) { int retval = 0; @@ -79,7 +79,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen, */ type = commit_type; } else if (S_ISDIR(mode)) { - if (show_recursive(base, baselen, pathname)) { + if (show_recursive(base->buf, base->len, pathname)) { retval = READ_TREE_RECURSIVE; if (!(ls_options & LS_SHOW_TREES)) return retval; @@ -90,7 +90,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen, return 0; if (chomp_prefix && - (baselen < chomp_prefix || memcmp(ls_tree_prefix, base, chomp_prefix))) + (base->len < chomp_prefix || + memcmp(ls_tree_prefix, base->buf, chomp_prefix))) return 0; if (!(ls_options & LS_NAME_ONLY)) { @@ -112,7 +113,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen, printf("%06o %s %s\t", mode, type, find_unique_abbrev(sha1, abbrev)); } - write_name_quotedpfx(base + chomp_prefix, baselen - chomp_prefix, + write_name_quotedpfx(base->buf + chomp_prefix, base->len - chomp_prefix, pathname, stdout, line_termination); return retval; } diff --git a/merge-recursive.c b/merge-recursive.c index fdb7d0f..25c067e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -275,23 +275,20 @@ struct tree *write_tree_from_memory(struct merge_options *o) } static int save_files_dirs(const unsigned char *sha1, - const char *base, int baselen, const char *path, + struct strbuf *base, const char *path, unsigned int mode, int stage, void *context) { - int len = strlen(path); - char *newpath = xmalloc(baselen + len + 1); + int baselen = base->len; struct merge_options *o = context; - memcpy(newpath, base, baselen); - memcpy(newpath + baselen, path, len); - newpath[baselen + len] = '\0'; + strbuf_addstr(base, path); if (S_ISDIR(mode)) - string_list_insert(&o->current_directory_set, newpath); + string_list_insert(&o->current_directory_set, base->buf); else - string_list_insert(&o->current_file_set, newpath); - free(newpath); + string_list_insert(&o->current_file_set, base->buf); + strbuf_setlen(base, baselen); return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0); } diff --git a/tree.c b/tree.c index bb02c1c..58ebfce 100644 --- a/tree.c +++ b/tree.c @@ -30,9 +30,12 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b return add_cache_entry(ce, opt); } -static int read_one_entry(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context) +static int read_one_entry(const unsigned char *sha1, struct strbuf *base, + const char *pathname, unsigned mode, int stage, + void *context) { - return read_one_entry_opt(sha1, base, baselen, pathname, mode, stage, + return read_one_entry_opt(sha1, base->buf, base->len, pathname, + mode, stage, ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); } @@ -40,9 +43,12 @@ static int read_one_entry(const unsigned char *sha1, const char *base, int basel * This is used when the caller knows there is no existing entries at * the stage that will conflict with the entry being added. */ -static int read_one_entry_quick(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context) +static int read_one_entry_quick(const unsigned char *sha1, struct strbuf *base, + const char *pathname, unsigned mode, int stage, + void *context) { - return read_one_entry_opt(sha1, base, baselen, pathname, mode, stage, + return read_one_entry_opt(sha1, base->buf, base->len, pathname, + mode, stage, ADD_CACHE_JUST_APPEND); } @@ -70,7 +76,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base, continue; } - switch (fn(entry.sha1, base->buf, base->len, + switch (fn(entry.sha1, base, entry.path, entry.mode, stage, context)) { case 0: continue; diff --git a/tree.h b/tree.h index d84ac63..d24125f 100644 --- a/tree.h +++ b/tree.h @@ -4,6 +4,7 @@ #include "object.h" extern const char *tree_type; +struct strbuf; struct tree { struct object object; @@ -22,7 +23,7 @@ void free_tree_buffer(struct tree *tree); struct tree *parse_tree_indirect(const unsigned char *sha1); #define READ_TREE_RECURSIVE 1 -typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const char *, unsigned int, int, void *); +typedef int (*read_tree_fn_t)(const unsigned char *, struct strbuf *, const char *, unsigned int, int, void *); extern int read_tree_recursive(struct tree *tree, const char *base, int baselen, -- 2.2.0.60.gb7b3c64 ``` ## Nguyễn Thái Ngọc Duy, 2014-11-30 09:05 Subject: [PATCH 2/3] ls-tree: remove path filtering logic in show_tree Message-ID: <1417338302-8208-3-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1417338302-8208-3-git-send-email-pclouds%40gmail.com In-Reply-To: <1417338302-8208-1-git-send-email-pclouds@gmail.com> ``` ls-tree uses read_tree_recursive() which already does path filtering using pathspec. No need to filter one more time based on prefix only. "ls-tree ../somewhere" does not work because of this. write_name_quotedpfx() can now be retired because nobody else uses it. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/ls-tree.c | 14 +++++++------- quote.c | 21 --------------------- quote.h | 2 -- t/t3102-ls-tree-wildcards.sh | 8 ++++++++ 4 files changed, 15 insertions(+), 30 deletions(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 1ab0381..053edb2 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -65,6 +65,7 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base, const char *pathname, unsigned mode, int stage, void *context) { int retval = 0; + int baselen; const char *type = blob_type; if (S_ISGITLINK(mode)) { @@ -89,11 +90,6 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base, else if (ls_options & LS_TREE_ONLY) return 0; - if (chomp_prefix && - (base->len < chomp_prefix || - memcmp(ls_tree_prefix, base->buf, chomp_prefix))) - return 0; - if (!(ls_options & LS_NAME_ONLY)) { if (ls_options & LS_SHOW_SIZE) { char size_text[24]; @@ -113,8 +109,12 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base, printf("%06o %s %s\t", mode, type, find_unique_abbrev(sha1, abbrev)); } - write_name_quotedpfx(base->buf + chomp_prefix, base->len - chomp_prefix, - pathname, stdout, line_termination); + baselen = base->len; + strbuf_addstr(base, pathname); + write_name_quoted_relative(base->buf, + chomp_prefix ? ls_tree_prefix : NULL, + stdout, line_termination); + strbuf_setlen(base, baselen); return retval; } diff --git a/quote.c b/quote.c index 45e3db1..7920e18 100644 --- a/quote.c +++ b/quote.c @@ -274,27 +274,6 @@ void write_name_quoted(const char *name, FILE *fp, int terminator) fputc(terminator, fp); } -void write_name_quotedpfx(const char *pfx, size_t pfxlen, - const char *name, FILE *fp, int terminator) -{ - int needquote = 0; - - if (terminator) { - needquote = next_quote_pos(pfx, pfxlen) < pfxlen - || name[next_quote_pos(name, -1)]; - } - if (needquote) { - fputc('"', fp); - quote_c_style_counted(pfx, pfxlen, NULL, fp, 1); - quote_c_style(name, NULL, fp, 1); - fputc('"', fp); - } else { - fwrite(pfx, pfxlen, 1, fp); - fputs(name, fp); - } - fputc(terminator, fp); -} - void write_name_quoted_relative(const char *name, const char *prefix, FILE *fp, int terminator) { diff --git a/quote.h b/quote.h index 71dcc3a..99e04d3 100644 --- a/quote.h +++ b/quote.h @@ -56,8 +56,6 @@ extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq extern void quote_two_c_style(struct strbuf *, const char *, const char *, int); extern void write_name_quoted(const char *name, FILE *, int terminator); -extern void write_name_quotedpfx(const char *pfx, size_t pfxlen, - const char *name, FILE *, int terminator); extern void write_name_quoted_relative(const char *name, const char *prefix, FILE *fp, int terminator); diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh index c286854..83fca8d 100755 --- a/t/t3102-ls-tree-wildcards.sh +++ b/t/t3102-ls-tree-wildcards.sh @@ -19,4 +19,12 @@ EOF test_cmp expected actual ' +test_expect_success 'ls-tree outside prefix' ' + cat >expected <actual && + test_cmp expected actual +' + test_done -- 2.2.0.60.gb7b3c64 ``` ## Nguyễn Thái Ngọc Duy, 2014-11-30 09:05 Subject: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported Message-ID: <1417338302-8208-4-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1417338302-8208-4-git-send-email-pclouds%40gmail.com In-Reply-To: <1417338302-8208-1-git-send-email-pclouds@gmail.com> ``` Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/ls-tree.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 053edb2..3b04a0f 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -174,7 +174,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) * cannot be lifted until it is converted to use * match_pathspec() or tree_entry_interesting() */ - parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE, + parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE | + PATHSPEC_EXCLUDE, PATHSPEC_PREFER_CWD, prefix, argv + 1); for (i = 0; i < pathspec.nr; i++) -- 2.2.0.60.gb7b3c64 ``` ## Junio C Hamano, 2014-12-01 19:32 Subject: Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base Message-ID: URL: https://gitlist.dev/e/xmqqk32bp3nk.fsf%40gitster.dls.corp.google.com In-Reply-To: <1417338302-8208-2-git-send-email-pclouds@gmail.com> ``` Nguyễn Thái Ngọc Duy writes: > This allows the callback to use 'base' as a temporary buffer to > quickly assemble full path "without" extra allocation. The callback > has to restore it afterwards of course. Hmph, what's the quote around 'without' doing there? ``` ## Junio C Hamano, 2014-12-01 19:40 Subject: Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported Message-ID: URL: https://gitlist.dev/e/xmqqfvczp39n.fsf%40gitster.dls.corp.google.com In-Reply-To: <1417338302-8208-4-git-send-email-pclouds@gmail.com> ``` Nguyễn Thái Ngọc Duy writes: > Signed-off-by: Nguyễn Thái Ngọc Duy Hmph, that's sad. Should the below say "test_expect_failure" without "test_must_fail", anticipating a fix later? t/t3102-ls-tree-wildcards.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh index 83fca8d..93127a0 100755 --- a/t/t3102-ls-tree-wildcards.sh +++ b/t/t3102-ls-tree-wildcards.sh @@ -27,4 +27,8 @@ EOF test_cmp expected actual ' +test_expect_success 'ls-tree rejects negated pathspec' ' + test_must_fail git ls-tree -r HEAD ":(exclude)a" "a*" +' + test_done ``` ## Duy Nguyen, 2014-12-02 12:11 Subject: Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base Message-ID: URL: https://gitlist.dev/e/CACsJy8B7G9xQerZ%2B6GTPQuG2v1DuJLXfsKFobuz_vyFahnXuYQ%40mail.gmail.com In-Reply-To: ``` On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> This allows the callback to use 'base' as a temporary buffer to >> quickly assemble full path "without" extra allocation. The callback >> has to restore it afterwards of course. > > Hmph, what's the quote around 'without' doing there? because it's only true if you haven't used up all preallocated space in strbuf. If someone passes an empty strbuf, then underneath strbuf may do a few realloc until the buffer is large enough. -- Duy ``` ## Duy Nguyen, 2014-12-02 12:14 Subject: Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported Message-ID: URL: https://gitlist.dev/e/CACsJy8CY0r0V7syGbtor2t%2BeOM4Tmzj38qL4EaL%2BQiE-z7B%2BAQ%40mail.gmail.com In-Reply-To: ``` On Tue, Dec 2, 2014 at 2:40 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Signed-off-by: Nguyễn Thái Ngọc Duy > > Hmph, that's sad. Should the below say "test_expect_failure" > without "test_must_fail", anticipating a fix later? Not a fix from me any time soon (I still need to improve pathspec support in git-mv). If the git-list-files series goes well, I do plan to make it list trees and it should support all pathspec without the fear of subtly breaking a plumbing. But I will change it to expect_failure unless you change your mind. -- Duy ``` ## Eric Sunshine, 2014-12-03 00:11 Subject: Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base Message-ID: URL: https://gitlist.dev/e/CAPig%2BcS1OK6pv5A0vuJf%3Dj6eFrNv70%3DAYgVz3zQny-md15_xKg%40mail.gmail.com In-Reply-To: ``` On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen wrote: > On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano wrote: >> Nguyễn Thái Ngọc Duy writes: >> >>> This allows the callback to use 'base' as a temporary buffer to >>> quickly assemble full path "without" extra allocation. The callback >>> has to restore it afterwards of course. >> >> Hmph, what's the quote around 'without' doing there? > > because it's only true if you haven't used up all preallocated space > in strbuf. If someone passes an empty strbuf, then underneath strbuf > may do a few realloc until the buffer is large enough. Would it be easier to understand if written like this? This allows the callback to use 'base' as a temporary buffer to quickly assemble full path, thus avoiding allocation/deallocation for each iteration step. ``` ## Junio C Hamano, 2014-12-03 16:13 Subject: Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base Message-ID: URL: https://gitlist.dev/e/xmqq1toglnjf.fsf%40gitster.dls.corp.google.com In-Reply-To: ``` Eric Sunshine writes: > On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen wrote: >> On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano wrote: >>> Nguyễn Thái Ngọc Duy writes: >>> >>>> This allows the callback to use 'base' as a temporary buffer to >>>> quickly assemble full path "without" extra allocation. The callback >>>> has to restore it afterwards of course. >>> >>> Hmph, what's the quote around 'without' doing there? >> >> because it's only true if you haven't used up all preallocated space >> in strbuf. If someone passes an empty strbuf, then underneath strbuf >> may do a few realloc until the buffer is large enough. > > Would it be easier to understand if written like this? > > This allows the callback to use 'base' as a temporary buffer to > quickly assemble full path, thus avoiding allocation/deallocation > for each iteration step. Thanks. I am not Duy so I do not know if that matches what he wanted to say, or if it matches what the change gives us. Your phrasing wouldn't have made me say "Hmph...?" and it is an improvement that goes in the right direction, I think. A question during the review, especially on proposed log messages and documentation changes, is rarely just a request to explain it to the questioner in the discussion. It is an indication that what is being commented on needs tweaking to be understood. ``` ## Duy Nguyen, 2014-12-08 13:30 Subject: Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base Message-ID: URL: https://gitlist.dev/e/CACsJy8BXjDqB0vGMw0QijaBKq0hsC8ZR4Teu%3DP7xUZFG7rjvag%40mail.gmail.com In-Reply-To: ``` On Wed, Dec 3, 2014 at 11:13 PM, Junio C Hamano wrote: > A question during the review, especially on proposed log messages > and documentation changes, is rarely just a request to explain it to > the questioner in the discussion. It is an indication that what is > being commented on needs tweaking to be understood. I do the same at work and somehow forgot to apply the same principle here :D Do you want to me resend with Eric's wording? -- Duy ```