{"thread":{"id":"59959","subject":"[PATCH] ls-tree: simplify prefix handling","startedAt":"2023-07-07T18:34:40Z","lastAt":"2023-07-07T19:02:11Z","messageCount":2,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479287","messageId":"b34aa99b-2bf1-c645-bdf5-03ac258b4bf5@web.de","threadId":"59959","inReplyTo":null,"subject":"[PATCH] ls-tree: simplify prefix handling","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-07-07T18:34:31Z","receivedAt":"2023-07-07T18:34:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"git ls-tree has two prefixes: The one handed to cmd_ls_tree(), i.e. the\ncurrent subdirectory in the repository (if any) and the \"display\" prefix\nused by the show_tree_*() functions.  The option --full-name clears the\nlast one, i.e. it shows full paths, and --full-tree clears both, i.e. it\nacts as if the command was started in the root of the repository.\n\nThe show_tree_*() functions use the ls_tree_options members chomp_prefix\nand ls_tree_prefix to determine their prefix values.  Calculate it once\nin cmd_ls_tree() instead, once the main prefix value is finalized.\n\nThis allows chomp_prefix to become a local variable.  Stop using\nstrlen(3) to determine its initial value -- we only care whether we got\na non-empty string, not precisely how long it is.\n\nRename ls_tree_prefix to prefix to demonstrate that we converted all\nusers and because the ls_tree_ part is no longer necessary since\n030a3d5d9e (ls-tree: use a \"struct options\", 2023-01-12) turned it from\na global variable to a struct member.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/ls-tree.c | 24 ++++++++++--------------\n 1 file changed, 10 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 7a062e2b67..53073d64cb 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -50,8 +50,7 @@ struct ls_tree_options {\n \t\tLS_SHOW_TREES = 1 << 2,\n \t} ls_options;\n \tstruct pathspec pathspec;\n-\tint chomp_prefix;\n-\tconst char *ls_tree_prefix;\n+\tconst char *prefix;\n \tconst char *format;\n };\n\n@@ -128,8 +127,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,\n \t\t\tstrbuf_add_unique_abbrev(&sb, oid, options->abbrev);\n \t\telse if (skip_prefix(format, \"(path)\", &format)) {\n \t\t\tconst char *name;\n-\t\t\tconst char *prefix = options->chomp_prefix ?\n-\t\t\t\t\t     options->ls_tree_prefix : NULL;\n+\t\t\tconst char *prefix = options->prefix;\n \t\t\tstruct strbuf sbuf = STRBUF_INIT;\n \t\t\tsize_t baselen = base->len;\n\n@@ -173,7 +171,7 @@ static void show_tree_common_default_long(struct ls_tree_options *options,\n \t\t\t\t\t  const char *pathname,\n \t\t\t\t\t  const size_t baselen)\n {\n-\tconst char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;\n+\tconst char *prefix = options->prefix;\n\n \tstrbuf_addstr(base, pathname);\n\n@@ -258,7 +256,7 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,\n \tif (early >= 0)\n \t\treturn early;\n\n-\tprefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;\n+\tprefix = options->prefix;\n \tstrbuf_addstr(base, pathname);\n \tif (options->null_termination) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n@@ -345,6 +343,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tstruct object_id oid;\n \tstruct tree *tree;\n \tint i, full_tree = 0;\n+\tint chomp_prefix = prefix && *prefix;\n \tread_tree_fn_t fn = NULL;\n \tenum ls_tree_cmdmode cmdmode = MODE_DEFAULT;\n \tint null_termination = 0;\n@@ -366,7 +365,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\t\t    MODE_NAME_STATUS),\n \t\tOPT_CMDMODE(0, \"object-only\", &cmdmode, N_(\"list only objects\"),\n \t\t\t    MODE_OBJECT_ONLY),\n-\t\tOPT_SET_INT(0, \"full-name\", &options.chomp_prefix,\n+\t\tOPT_SET_INT(0, \"full-name\", &chomp_prefix,\n \t\t\t    N_(\"use full path names\"), 0),\n \t\tOPT_BOOL(0, \"full-tree\", &full_tree,\n \t\t\t N_(\"list entire tree; not just current directory \"\n@@ -381,18 +380,15 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \tint ret;\n\n \tgit_config(git_default_config, NULL);\n-\toptions.ls_tree_prefix = prefix;\n-\tif (prefix)\n-\t\toptions.chomp_prefix = strlen(prefix);\n\n \targc = parse_options(argc, argv, prefix, ls_tree_options,\n \t\t\t     ls_tree_usage, 0);\n \toptions.null_termination = null_termination;\n\n-\tif (full_tree) {\n-\t\toptions.ls_tree_prefix = prefix = NULL;\n-\t\toptions.chomp_prefix = 0;\n-\t}\n+\tif (full_tree)\n+\t\tprefix = NULL;\n+\toptions.prefix = chomp_prefix ? prefix : NULL;\n+\n \t/*\n \t * We wanted to detect conflicts between --name-only and\n \t * --name-status, but once we're done with that subsequent\n--\n2.41.0\n"},{"id":"479290","messageId":"xmqqh6qf8s1w.fsf@gitster.g","threadId":"59959","inReplyTo":"b34aa99b-2bf1-c645-bdf5-03ac258b4bf5@web.de","subject":"Re: [PATCH] ls-tree: simplify prefix handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-07T19:01:15Z","receivedAt":"2023-07-07T19:02:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> git ls-tree has two prefixes: The one handed to cmd_ls_tree(), i.e. the\n> current subdirectory in the repository (if any) and the \"display\" prefix\n> used by the show_tree_*() functions.  The option --full-name clears the\n> last one, i.e. it shows full paths, and --full-tree clears both, i.e. it\n> acts as if the command was started in the root of the repository.\n\nSeeing \"there are two\" and then \"the last one\" made me come back to\nthe beginning to see if I missed the third option, as I am not used\nto see the second one among two called \"the last\", even though it is\nlogically correct.  \"clears the latter\", perhaps, as \"former vs\nlatter\" is sensible only when there are two, may have avoided my\nconfusion, but that is minor.\n\n> The show_tree_*() functions use the ls_tree_options members chomp_prefix\n> and ls_tree_prefix to determine their prefix values.  Calculate it once\n> in cmd_ls_tree() instead, once the main prefix value is finalized.\n\nVery nice observation and simplification.\n\nThanks.\n"}]}