On 2022.03.23 17:13, Teng Long wrote:
> From: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
>
> Make the various if/else in the callbacks for the "fast path" a lot
> easier to read by just using common functions for the parts that are
> common, and have per-format callbacks for those parts that are
> different.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx>
> ---
> builtin/ls-tree.c | 199 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 125 insertions(+), 74 deletions(-)
>
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index 6550f27dfe..44a91cf9d0 100644
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -173,108 +173,157 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
> return recurse;
> }
>
> -static int show_default(struct show_tree_data *data)
> +static int show_tree_common(struct show_tree_data *data, int *recurse,
> + const struct object_id *oid, struct strbuf *base,
> + const char *pathname, unsigned mode)
> {
> - size_t baselen = data->base->len;
> -
> - if (cmdmode == MODE_LONG) {
> - char size_text[24];
> - if (data->type == OBJ_BLOB) {
> - unsigned long size;
> - if (oid_object_info(the_repository, data->oid, &size) == OBJ_BAD)
> - xsnprintf(size_text, sizeof(size_text), "BAD");
> - else
> - xsnprintf(size_text, sizeof(size_text),
> - "%" PRIuMAX, (uintmax_t)size);
> - } else {
> - xsnprintf(size_text, sizeof(size_text), "-");
> - }
> - printf("%06o %s %s %7s\t", data->mode, type_name(data->type),
> - find_unique_abbrev(data->oid, abbrev), size_text);
> - } else {
> - printf("%06o %s %s\t", data->mode, type_name(data->type),
> - find_unique_abbrev(data->oid, abbrev));
> - }
> - baselen = data->base->len;
> - strbuf_addstr(data->base, data->pathname);
> - write_name_quoted_relative(data->base->buf,
> - chomp_prefix ? ls_tree_prefix : NULL, stdout,
> - line_termination);
> - strbuf_setlen(data->base, baselen);
> - return 1;
> -}
> -
> -static int show_tree(const struct object_id *oid, struct strbuf *base,
> - const char *pathname, unsigned mode, void *context)
> -{
> - int recurse = 0;
> - size_t baselen;
> enum object_type type = object_type(mode);
> - struct show_tree_data data = {
> - .mode = mode,
> - .type = type,
> - .oid = oid,
> - .pathname = pathname,
> - .base = base,
> - };
> + int ret = -1;
> +
> + *recurse = 0;
> + data->mode = mode;
> + data->type = type;
> + data->oid = oid;
> + data->pathname = pathname;
> + data->base = base;
>
> if (type == OBJ_BLOB) {
> if (ls_options & LS_TREE_ONLY)
> - return 0;
> + ret = 0;
> } else if (type == OBJ_TREE &&
> show_recursive(base->buf, base->len, pathname)) {
> - recurse = READ_TREE_RECURSIVE;
> + *recurse = READ_TREE_RECURSIVE;
> if (!(ls_options & LS_SHOW_TREES))
> - return recurse;
> + ret = *recurse;
> }
>
> - if (cmdmode == MODE_OBJECT_ONLY) {
> - printf("%s%c", find_unique_abbrev(oid, abbrev), line_termination);
> - return recurse;
> - }
> + return ret;
> +}
>
> - if (cmdmode == MODE_NAME_ONLY) {
> - 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 recurse;
> +static void show_tree_common_default_long(struct strbuf *base,
> + const char *pathname,
> + const size_t baselen)
> +{
> + strbuf_addstr(base, pathname);
> + write_name_quoted_relative(base->buf,
> + chomp_prefix ? ls_tree_prefix : NULL, stdout,
> + line_termination);
> + strbuf_setlen(base, baselen);
> +}
> +
> +static int show_tree_default(const struct object_id *oid, struct strbuf *base,
> + const char *pathname, unsigned mode,
> + void *context)
> +{
> + int early;
> + int recurse;
> + struct show_tree_data data = { 0 };
> +
> + early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
> + if (early >= 0)
> + return early;
> +
> + printf("%06o %s %s\t", data.mode, type_name(data.type),
> + find_unique_abbrev(data.oid, abbrev));
> + show_tree_common_default_long(base, pathname, data.base->len);
> + return recurse;
> +}
> +
> +static int show_tree_long(const struct object_id *oid, struct strbuf *base,
> + const char *pathname, unsigned mode, void *context)
> +{
> + int early;
> + int recurse;
> + struct show_tree_data data = { 0 };
> + char size_text[24];
> +
> + early = show_tree_common(&data, &recurse, oid, base, pathname, mode);
> + if (early >= 0)
> + return early;
> +
> + if (data.type == OBJ_BLOB) {
> + unsigned long size;
> + if (oid_object_info(the_repository, data.oid, &size) == OBJ_BAD)
> + xsnprintf(size_text, sizeof(size_text), "BAD");
> + else
> + xsnprintf(size_text, sizeof(size_text),
> + "%" PRIuMAX, (uintmax_t)size);
> + } else {
> + xsnprintf(size_text, sizeof(size_text), "-");
> }
>
> - if (cmdmode == MODE_LONG ||
> - (!ls_options || (ls_options & LS_RECURSIVE)
> - || (ls_options & LS_SHOW_TREES)
> - || (ls_options & LS_TREE_ONLY)))
> - show_default(&data);
> + printf("%06o %s %s %7s\t", data.mode, type_name(data.type),
> + find_unique_abbrev(data.oid, abbrev), size_text);
> + show_tree_common_default_long(base, pathname, data.base->len);
> + return 1;
> +}
This commit makes `git ls-tree -l ...` list files recursively,
regardless of whether `-r` was provided or not. I believe it's due to
show_tree_long() returning 1 instead of the value of recurse. Is there a
reason why we unconditionally return 1 here?
