git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v14 15/15] ls-tree: split up "fast path" callbacks

From
Josh Steadmon <steadmon@google.com>
Date
Apr 4, 2022, 20:06 UTC
Message-ID
<YktP0LltNKq7JZZs@google.com>
In-Reply-To
<55a96e933b0949bbf3f33153b8e370af6c6299bb.1648026472.git.dyroneteng@gmail.com>
On 2022.03.23 17:13, Teng Long wrote:
Show 167 quoted lines
> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> 
> 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@gmail.com>
> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  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?

Previous: Teng LongNext: Josh Steadmon
Message 83 of 107 in “ls-tree: "--object-only" and "--format" opts”
  1. 00/13 ls-tree: "--object-only" and "--format" optsTeng Long, Feb 8, 2022
  2. 01/13 ls-tree: remove commented-out codeTeng Long, Feb 8, 2022
  3. 03/13 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Feb 8, 2022
  4. 12/13 ls-tree: introduce function "fast_path()"Teng Long, Feb 8, 2022
  5. Ævar Arnfjörð BjarmasonFeb 19, 2022
  6. 13/13 ls-tree.c: support --object-only option for "git-ls-tree"Teng Long, Feb 8, 2022
  7. Ævar Arnfjörð BjarmasonFeb 19, 2022
  8. 04/13 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Feb 8, 2022
  9. 02/13 ls-tree: add missing braces to "else" armsTeng Long, Feb 8, 2022
  10. 06/13 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Feb 8, 2022
  11. Ævar Arnfjörð BjarmasonFeb 19, 2022
  12. 05/13 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Feb 8, 2022
  13. 07/13 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Feb 8, 2022
  14. Ævar Arnfjörð BjarmasonFeb 19, 2022
  15. 08/13 ls-tree: slightly refactor `show_tree()`Teng Long, Feb 8, 2022
  16. Ævar Arnfjörð BjarmasonFeb 19, 2022
  17. 10/13 cocci: allow padding with `strbuf_addf()`Teng Long, Feb 8, 2022
  18. 09/13 ls-tree: introduce struct "show_tree_data"Teng Long, Feb 8, 2022
  19. 11/13 ls-tree.c: introduce "--format" optionTeng Long, Feb 8, 2022
  20. Ævar Arnfjörð BjarmasonFeb 19, 2022
  21. 00/12 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 4, 2022
  22. 01/12 ls-tree: remove commented-out codeTeng Long, Mar 4, 2022
  23. 02/12 ls-tree: add missing braces to "else" armsTeng Long, Mar 4, 2022
  24. 03/12 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 4, 2022
  25. 04/12 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 4, 2022
  26. 05/12 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 4, 2022
  27. 06/12 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 4, 2022
  28. 07/12 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 4, 2022
  29. 08/12 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 4, 2022
  30. 09/12 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 4, 2022
  31. 10/12 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 4, 2022
  32. 11/12 ls-tree: introduce "--format" optionTeng Long, Mar 4, 2022
  33. 12/12 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 4, 2022
  34. 0/7 fixups/suggestions/musings for tl/ls-tree-oid-onlyÆvar Arnfjörð Bjarmason, Mar 10, 2022
  35. 1/7 ls-tree tests: add tests for --name-statusÆvar Arnfjörð Bjarmason, Mar 10, 2022
  36. 2/7 ls-tree tests: exhaustively test fast & slow path for --formatÆvar Arnfjörð Bjarmason, Mar 10, 2022
  37. 5/7 ls-tree: detect and error on --name-only --name-statusÆvar Arnfjörð Bjarmason, Mar 10, 2022
  38. 4/7 ls-tree: remove unused "MODE_UNSPECIFIED"Ævar Arnfjörð Bjarmason, Mar 10, 2022
  39. 3/7 ls-tree: remove dead labelsÆvar Arnfjörð Bjarmason, Mar 10, 2022
  40. 6/7 ls-tree: remove FIELD_*, just use MODE_*Ævar Arnfjörð Bjarmason, Mar 10, 2022
  41. 7/7 ls-tree: split up "fast path" callbacksÆvar Arnfjörð Bjarmason, Mar 10, 2022
  42. Teng LongMar 17, 2022
  43. Ævar Arnfjörð BjarmasonMar 17, 2022
  44. 00/16 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 21, 2022
  45. 01/16 ls-tree: remove commented-out codeTeng Long, Mar 21, 2022
  46. 02/16 ls-tree: add missing braces to "else" armsTeng Long, Mar 21, 2022
  47. 03/16 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 21, 2022
  48. 04/16 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 21, 2022
  49. 05/16 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 21, 2022
  50. 06/16 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 21, 2022
  51. 07/16 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 21, 2022
  52. 08/16 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 21, 2022
  53. 09/16 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 21, 2022
  54. 10/16 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 21, 2022
  55. 11/16 ls-tree: introduce "--format" optionTeng Long, Mar 21, 2022
  56. Ævar Arnfjörð BjarmasonMar 21, 2022
  57. 12/16 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 21, 2022
  58. 13/16 ls-tree tests: add tests for --name-statusTeng Long, Mar 21, 2022
  59. Ævar Arnfjörð BjarmasonMar 21, 2022
  60. 14/16 ls-tree: detect and error on --name-only --name-statusTeng Long, Mar 21, 2022
  61. 15/16 ls-tree: remove FIELD_*, just use MODE_*Teng Long, Mar 21, 2022
  62. Ævar Arnfjörð BjarmasonMar 21, 2022
  63. 16/16 ls-tree: split up "fast path" callbacksTeng Long, Mar 21, 2022
  64. Ævar Arnfjörð BjarmasonMar 21, 2022
  65. Teng LongMar 23, 2022
  66. Junio C HamanoMar 21, 2022
  67. 00/15 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 23, 2022
  68. 01/15 ls-tree tests: add tests for --name-statusTeng Long, Mar 23, 2022
  69. 02/15 ls-tree: remove commented-out codeTeng Long, Mar 23, 2022
  70. 03/15 ls-tree: add missing braces to "else" armsTeng Long, Mar 23, 2022
  71. 04/15 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 23, 2022
  72. 05/15 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 23, 2022
  73. 06/15 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 23, 2022
  74. 07/15 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 23, 2022
  75. 08/15 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 23, 2022
  76. 09/15 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 23, 2022
  77. 10/15 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 23, 2022
  78. 11/15 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 23, 2022
  79. 12/15 ls-tree: introduce "--format" optionTeng Long, Mar 23, 2022
  80. 13/15 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 23, 2022
  81. 14/15 ls-tree: detect and error on --name-only --name-statusTeng Long, Mar 23, 2022
  82. 15/15 ls-tree: split up "fast path" callbacksTeng Long, Mar 23, 2022
  83. Josh SteadmonApr 4, 2022
  84. ls-tree: `-l` should not imply recursive listingJosh Steadmon, Apr 4, 2022
  85. ls-tree: fix --long implying -r regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, Apr 4, 2022
  86. Junio C HamanoApr 6, 2022
  87. Ævar Arnfjörð BjarmasonApr 6, 2022
  88. Junio C HamanoApr 6, 2022
  89. Ævar Arnfjörð BjarmasonApr 7, 2022
  90. Junio C HamanoApr 7, 2022
  91. ls-tree: test for the regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, May 31, 2022
  92. Johannes SchindelinJun 2, 2022
  93. Junio C HamanoJun 2, 2022
  94. js/ci-github-workflow-markup output regression (was: [PATCH] ls-tree: test for the regression in 9c4d58ff2c3)Ævar Arnfjörð Bjarmason, Jun 3, 2022
  95. Junio C HamanoJun 3, 2022
  96. Ævar Arnfjörð BjarmasonJun 3, 2022
  97. Junio C HamanoJun 7, 2022
  98. Ævar Arnfjörð BjarmasonJun 7, 2022
  99. Johannes SchindelinJun 8, 2022
  100. Ævar Arnfjörð BjarmasonJun 9, 2022
  101. ls-tree: test for the regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, Jun 3, 2022
  102. Johannes SchindelinJun 8, 2022
  103. Teng LongApr 7, 2022
  104. Junio C HamanoApr 6, 2022
  105. Junio C HamanoMar 23, 2022
  106. Teng LongMar 24, 2022
  107. Ævar Arnfjörð BjarmasonFeb 28, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.