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

Re: [PATCH v10 5/9] ls-tree: optimize naming and handling of "return" in show_tree()

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jan 13, 2022, 06:49 UTC
Message-ID
<220113.861r1cxgxx.gmgdl@evledraar.gmail.com>
In-Reply-To
<b04188c822c32aab6ef59099a0c9078aeda065c3.1641978175.git.dyroneteng@gmail.com>
On Thu, Jan 13 2022, Teng Long wrote:

Re the $subject: Is "optimize naming" here just referring to the s/retval/recurse/g?

Personally I think just a s/retval/ret/g here would make more senes if we're doing any change at all, and in either case having this variable re-rename split up as its own commit would make the proposed control flow changes clearer.

Show 32 quoted lines
> The variable which "show_tree()" return is named "retval", a name that's
> a little hard to understand. This commit tries to make the variable
> and the related codes more clear in the context.
>
> The commit firstly rename "retval" to "recurse" which is a more
> meaningful name than before. Secondly, "get_type()" is introduced
> to setup the "type" by "mode", this will remove some of the nested if.
> After this, The codes here become a little bit clearer, so we do not
> need to take a look at "read_tree_at()" in "tree.c" to make sure the
> context of the return value.
>
> Signed-off-by: Teng Long <dyronetengb@gmail.com>
> ---
>  builtin/ls-tree.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index eecc7482d5..9729854a3d 100644
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -61,24 +61,27 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname
>  	return 0;
>  }
>  
> +static enum object_type get_type(unsigned int mode)
> +{
> +	return (S_ISGITLINK(mode)
> +	        ? OBJ_COMMIT
> +	        : S_ISDIR(mode)
> +	        ? OBJ_TREE
> +	        : OBJ_BLOB);
> +}

This new function is a re-invention of the object_type() utility in cache.h, and isn't needed. I.e....

Show 20 quoted lines
>  static int show_tree(const struct object_id *oid, struct strbuf *base,
>  		const char *pathname, unsigned mode, void *context)
>  {
> -	int retval = 0;
> +	int recurse = 0;
>  	size_t baselen;
> -	enum object_type type = OBJ_BLOB;
> -
> -	if (S_ISGITLINK(mode)) {
> -		type = OBJ_COMMIT;
> -	} else if (S_ISDIR(mode)) {
> -		if (show_recursive(base->buf, base->len, pathname)) {
> -			retval = READ_TREE_RECURSIVE;
> -			if (!(ls_options & LS_SHOW_TREES))
> -				return retval;
> -		}
> -		type = OBJ_TREE;
> -	}
> -	else if (ls_options & LS_TREE_ONLY)
> +	enum object_type type = get_type(mode);
...just drop it and do this:
	-       enum object_type type = get_type(mode);
	+       enum object_type type = object_type(mode);
Previous: Teng LongNext: Teng Long
Message 58 of 74 in “ls-tree: "--object-only" and "--format" opts”
  1. 0/8 ls-tree: "--object-only" and "--format" optsTeng Long, Jan 1, 2022
  2. 1/8 ls-tree: remove commented-out codeTeng Long, Jan 1, 2022
  3. 2/8 ls-tree: add missing braces to "else" armsTeng Long, Jan 1, 2022
  4. 3/8 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Jan 1, 2022
  5. 4/8 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Jan 1, 2022
  6. 5/8 ls-tree: split up the "init" part of show_tree()Teng Long, Jan 1, 2022
  7. Junio C HamanoJan 4, 2022
  8. Teng LongJan 4, 2022
  9. 6/8 ls-tree.c: support --object-only option for "git-ls-tree"Teng Long, Jan 1, 2022
  10. Junio C HamanoJan 4, 2022
  11. Teng LongJan 4, 2022
  12. 7/8 ls-tree.c: introduce struct "shown_data"Teng Long, Jan 1, 2022
  13. Junio C HamanoJan 3, 2022
  14. Teng LongJan 4, 2022
  15. 8/8 ls-tree.c: introduce "--format" optionTeng Long, Jan 1, 2022
  16. Johannes SchindelinJan 4, 2022
  17. Johannes SchindelinJan 4, 2022
  18. Teng LongJan 5, 2022
  19. Teng LongJan 5, 2022
  20. Johannes SchindelinJan 5, 2022
  21. Teng LongJan 5, 2022
  22. 0/9 ls-tree.c: introduce "--format" optionTeng Long, Jan 6, 2022
  23. 1/9 ls-tree: remove commented-out codeTeng Long, Jan 6, 2022
  24. 2/9 ls-tree: add missing braces to "else" armsTeng Long, Jan 6, 2022
  25. 3/9 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Jan 6, 2022
  26. 4/9 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Jan 6, 2022
  27. 5/9 ls-tree: optimize naming and handling of "return" in show_tree()Teng Long, Jan 6, 2022
  28. Junio C HamanoJan 6, 2022
  29. Teng LongJan 11, 2022
  30. 6/9 ls-tree.c: support --object-only option for "git-ls-tree"Teng Long, Jan 6, 2022
  31. 7/9 ls-tree.c: introduce struct "show_tree_data"Teng Long, Jan 6, 2022
  32. 8/9 ls-tree.c: introduce "--format" optionTeng Long, Jan 6, 2022
  33. Martin ÅgrenJan 10, 2022
  34. Teng LongJan 11, 2022
  35. 9/9 cocci: allow padding with `strbuf_addf()`Teng Long, Jan 6, 2022
  36. Johannes SchindelinJan 7, 2022
  37. Teng LongJan 10, 2022
  38. Johannes SchindelinJan 10, 2022
  39. Teng LongJan 10, 2022
  40. Junio C HamanoJan 10, 2022
  41. Ævar Arnfjörð BjarmasonJan 10, 2022
  42. Junio C HamanoJan 10, 2022
  43. Ævar Arnfjörð BjarmasonJan 10, 2022
  44. Teng LongJan 11, 2022
  45. Taylor BlauJan 11, 2022
  46. René ScharfeJan 11, 2022
  47. Taylor BlauJan 11, 2022
  48. Teng LongJan 13, 2022
  49. Ævar Arnfjörð BjarmasonJan 11, 2022
  50. Teng LongJan 13, 2022
  51. Teng LongJan 13, 2022
  52. 0/9 ls-tree: "--object-only" and "--format" optsTeng Long, Jan 13, 2022
  53. 1/9 ls-tree: remove commented-out codeTeng Long, Jan 13, 2022
  54. 2/9 ls-tree: add missing braces to "else" armsTeng Long, Jan 13, 2022
  55. 3/9 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Jan 13, 2022
  56. 4/9 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Jan 13, 2022
  57. 5/9 ls-tree: optimize naming and handling of "return" in show_tree()Teng Long, Jan 13, 2022
  58. Ævar Arnfjörð BjarmasonJan 13, 2022
  59. Teng LongJan 14, 2022
  60. Ævar Arnfjörð BjarmasonJan 14, 2022
  61. 6/9 ls-tree.c: support --object-only option for "git-ls-tree"Teng Long, Jan 13, 2022
  62. Ævar Arnfjörð BjarmasonJan 13, 2022
  63. Teng LongJan 14, 2022
  64. Ævar Arnfjörð BjarmasonJan 14, 2022
  65. Teng LongJan 18, 2022
  66. Ævar Arnfjörð BjarmasonFeb 4, 2022
  67. Teng LongFeb 7, 2022
  68. 7/9 ls-tree.c: introduce struct "show_tree_data"Teng Long, Jan 13, 2022
  69. Ævar Arnfjörð BjarmasonJan 13, 2022
  70. Teng LongJan 14, 2022
  71. 8/9 cocci: allow padding with `strbuf_addf()`Teng Long, Jan 13, 2022
  72. 9/9 ls-tree.c: introduce "--format" optionTeng Long, Jan 13, 2022
  73. Ævar Arnfjörð BjarmasonJan 13, 2022
  74. Teng LongJan 18, 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.