{"thread":{"id":"60072","subject":"[PATCH] ls-tree: default to HEAD","startedAt":"2023-08-07T12:49:24Z","lastAt":"2023-08-11T18:22:39Z","messageCount":4,"participants":["Ryan Williams via GitGitGadget","Junio C Hamano","Taylor Blau","Linus Arver"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"480212","messageId":"pull.1566.git.1691412557518.gitgitgadget@gmail.com","threadId":"60072","inReplyTo":null,"subject":"[PATCH] ls-tree: default to HEAD","fromName":"Ryan Williams via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-07T12:49:17Z","receivedAt":"2023-08-07T12:49:24Z","isPatch":true,"sender":{"key":"name:Ryan Williams","avatar":null},"body":"From: Ryan Williams \n\nWhen no positional arguments are passed to `git ls-tree`, it currently\nprints \"usage\" info to stderr and exits with code 129. A more intuitive\ndefault would be to operate on the `HEAD` commit's tree (similarly to\n`git show`, `git log`, and possibly others).\n\nThis patch updates `git ls-tree [options...]` to operate identically to\n`git ls-tree [options...] HEAD`, updates the docs to reflect that\n`` is optional (and `[path...]` args can only be provided if a\n`` is explicitly provided first), and duplicates some existing\ntest cases to omit the `HEAD` argument to `ls-tree` (verifying that\n`ls-tree` behaves identically whether `HEAD` is provided or not).\n\nSigned-off-by: Ryan Williams \n---\n ls-tree: default to HEAD\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1566%2Frunsascoded%2Fls-tree-head-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1566/runsascoded/ls-tree-head-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1566\n\n Documentation/git-ls-tree.txt | 6 +++---\n builtin/ls-tree.c | 11 ++++++++---\n t/t3105-ls-tree-output.sh | 7 ++++++-\n 3 files changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-ls-tree.txt b/Documentation/git-ls-tree.txt\nindex 6572095d8d6..6211d630974 100644\n--- a/Documentation/git-ls-tree.txt\n+++ b/Documentation/git-ls-tree.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n [verse]\n 'git ls-tree' [-d] [-r] [-t] [-l] [-z]\n \t [--name-only] [--name-status] [--object-only] [--full-name] [--full-tree] [--abbrev[=]] [--format=]\n-\t [...]\n+\t [ [...]]\n \n DESCRIPTION\n -----------\n@@ -36,7 +36,7 @@ in the current working directory. Note that:\n OPTIONS\n -------\n ::\n-\tId of a tree-ish.\n+\tId of a tree-ish. If omitted, defaults to \"HEAD\".\n \n -d::\n \tShow only the named tree entry itself, not its children.\n@@ -139,7 +139,7 @@ which is able to interpolate different fields using a `%(fieldname)` notation.\n For example, if you only care about the \"objectname\" and \"path\" fields, you\n can execute with a specific \"--format\" like\n \n-\tgit ls-tree --format='%(objectname) %(path)' \n+\tgit ls-tree --format='%(objectname) %(path)' []\n \n FIELD NAMES\n -----------\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex f558db5f3b8..b1e337ccde9 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -18,7 +18,7 @@\n #include \"pathspec.h\"\n \n static const char * const ls_tree_usage[] = {\n-\tN_(\"git ls-tree [] [...]\"),\n+\tN_(\"git ls-tree [] [ [...]]\"),\n \tNULL\n };\n \n@@ -377,6 +377,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t};\n \tstruct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format;\n \tint ret;\n+\t/* If no positional args were passed, default to HEAD. */\n+\tconst char *fallback_args[] = { \"HEAD\", NULL };\n \n \tgit_config(git_default_config, NULL);\n \n@@ -405,8 +407,11 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t\tusage_msg_opt(\n \t\t\t_(\"--format can't be combined with other format-altering options\"),\n \t\t\tls_tree_usage, ls_tree_options);\n-\tif (argc < 1)\n-\t\tusage_with_options(ls_tree_usage, ls_tree_options);\n+\tif (argc < 1) {\n+\t\t/* `git ls-tree [flags...]` -> `git ls-tree [flags...] HEAD`. */\n+\t\targv = fallback_args;\n+\t\targc = 1;\n+\t}\n \tif (repo_get_oid(the_repository, argv[0], &oid))\n \t\tdie(\"Not a valid object name %s\", argv[0]);\n \ndiff --git a/t/t3105-ls-tree-output.sh b/t/t3105-ls-tree-output.sh\nindex ce2391e28be..cb05529c0ad 100755\n--- a/t/t3105-ls-tree-output.sh\n+++ b/t/t3105-ls-tree-output.sh\n@@ -26,11 +26,16 @@ test_ls_tree_format_mode_output () {\n \t\tlocal mode=\"$1\" &&\n \t\tshift &&\n \n-\t\ttest_expect_success \"'ls-tree $opts${mode:+ $mode}' output\" '\n+\t\ttest_expect_success \"'ls-tree ${mode:+$mode }$opts' output\" '\n \t\t\tgit ls-tree ${mode:+$mode }$opts HEAD >actual &&\n \t\t\ttest_cmp expect actual\n \t\t'\n \n+\t\ttest_expect_success \"'ls-tree ${mode:+$mode }$opts' (default HEAD) output\" '\n+\t\t\tgit ls-tree ${mode:+$mode }$opts >actual &&\n+\t\t\ttest_cmp expect actual\n+\t\t'\n+\n \t\tcase \"$opts\" in\n \t\t--full-tree)\n \t\t\ttest_expect_success \"'ls-tree $opts${mode:+ $mode}' output (via subdir, fails)\" '\n\nbase-commit: ac83bc5054c2ac489166072334b4147ce6d0fccb\n-- \ngitgitgadget\n"},{"id":"480220","messageId":"xmqqmsz2hl9m.fsf@gitster.g","threadId":"60072","inReplyTo":"pull.1566.git.1691412557518.gitgitgadget@gmail.com","subject":"Re: [PATCH] ls-tree: default to HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-07T16:25:57Z","receivedAt":"2023-08-07T16:26:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ryan Williams via GitGitGadget\" writes:\n\n> From: Ryan Williams \n>\n> When no positional arguments are passed to `git ls-tree`, it currently\n> prints \"usage\" info to stderr and exits with code 129. A more intuitive\n> default would be to operate on the `HEAD` commit's tree (similarly to\n> `git show`, `git log`, and possibly others).\n\nAs 'ls-tree' is a plumbing command meant for script writers, it was\ndesigned to require the users to be more explicit. So, similarity\nto \"show\" and other Porcelain commands do not weigh much here. Same\nfor \"rev-list\" that does not fall back to HEAD.\n\nThis was a very deliberate design decision to help use of the\nplumbing commands. A buggy script may say\n\n TREE=$(some command to find a tree object)\n git ls-tree $TREE\n\nwithout making sure something sensible is in $TREE (and not even\nquoting it like \"$TREE\"), and if ls-tree defaulted to something, the\nscript will silently produce a wrong result, instead of failing,\nrobbing the script writer a chance to notice a bug in their code to\ncome up with the TREE object name.\n\nSo, I dunno.\n\nThanks.\n"},{"id":"480559","messageId":"ZNZxWdUHMwGxWdbJ@nand.local","threadId":"60072","inReplyTo":"xmqqmsz2hl9m.fsf@gitster.g","subject":"Re: [PATCH] ls-tree: default to HEAD","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-08-11T17:35:21Z","receivedAt":"2023-08-11T17:35:37Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Aug 07, 2023 at 09:25:57AM -0700, Junio C Hamano wrote:\n> \"Ryan Williams via GitGitGadget\" writes:\n>\n> > From: Ryan Williams \n> >\n> > When no positional arguments are passed to `git ls-tree`, it currently\n> > prints \"usage\" info to stderr and exits with code 129. A more intuitive\n> > default would be to operate on the `HEAD` commit's tree (similarly to\n> > `git show`, `git log`, and possibly others).\n>\n> As 'ls-tree' is a plumbing command meant for script writers, it was\n> designed to require the users to be more explicit. So, similarity\n> to \"show\" and other Porcelain commands do not weigh much here. Same\n> for \"rev-list\" that does not fall back to HEAD.\n>\n> This was a very deliberate design decision to help use of the\n> plumbing commands. A buggy script may say\n>\n> TREE=$(some command to find a tree object)\n> git ls-tree $TREE\n>\n> without making sure something sensible is in $TREE (and not even\n> quoting it like \"$TREE\"), and if ls-tree defaulted to something, the\n> script will silently produce a wrong result, instead of failing,\n> robbing the script writer a chance to notice a bug in their code to\n> come up with the TREE object name.\n\nYeah, I am similarly torn. On one hand, I think that it is true that\npeople use ls-tree for various tasks more than we'd expect from a\ntypical plumbing command (but less than a porcelain one, to be sure).\n\nSo in that sense I am inclined to agree with Ryan that their suggestion\nis a good one. ...But, I think even more important than that is avoiding\nbuggy invocations like the kind you describe above that would produce\nsubtly wrong results.\n\nI think you could make an argument that you could implement that\nbehavior and also emit a warning() when tree-ish is blank and the output\nisn't going to a TTY. But that seems kind of gross to me, so I'd just as\nsoon not change the existing behavior here.\n\nThanks,\nTaylor\n"},{"id":"480572","messageId":"owly5y5lfnh1.fsf@fine.c.googlers.com","threadId":"60072","inReplyTo":"xmqqmsz2hl9m.fsf@gitster.g","subject":"Re: [PATCH] ls-tree: default to HEAD","fromName":"Linus Arver","fromEmail":"linusa@google.com","sentAt":"2023-08-11T18:22:34Z","receivedAt":"2023-08-11T18:22:39Z","isPatch":true,"sender":{"key":"linus@ucla.edu","avatar":null},"body":"Junio C Hamano writes:\n\n> \"Ryan Williams via GitGitGadget\" writes:\n>\n>> From: Ryan Williams \n>>\n>> When no positional arguments are passed to `git ls-tree`, it currently\n>> prints \"usage\" info to stderr and exits with code 129. A more intuitive\n>> default would be to operate on the `HEAD` commit's tree (similarly to\n>> `git show`, `git log`, and possibly others).\n>\n> As 'ls-tree' is a plumbing command meant for script writers, it was\n> designed to require the users to be more explicit. So, similarity\n> to \"show\" and other Porcelain commands do not weigh much here. Same\n> for \"rev-list\" that does not fall back to HEAD.\n\nIndeed, we already partition these commands into these porcelain vs\nplumbing categories in \"man git\". There, we say for plumbing\n\n The interface (input, output, set of options and the semantics) to\n these low-level commands are meant to be a lot more stable than\n Porcelain level commands, because these commands are primarily for\n scripted use. The interface to Porcelain commands on the other hand\n are subject to change in order to improve the end user experience.\n\n> This was a very deliberate design decision to help use of the\n> plumbing commands. A buggy script may say\n>\n> TREE=$(some command to find a tree object)\n> git ls-tree $TREE\n>\n> without making sure something sensible is in $TREE (and not even\n> quoting it like \"$TREE\"), and if ls-tree defaulted to something, the\n> script will silently produce a wrong result, instead of failing,\n> robbing the script writer a chance to notice a bug in their code to\n> come up with the TREE object name.\n\nI think this illustrative example (at least the general language of it,\nif not the entire example) could be added to the referenced section of\nour docs (I'd be happy to add this).\n\nOn a related note, does it make sense to explicitly call out the\nplumbing vs porcelain distinction in the individual manpages for each\ncommand? For example, I grepped for \"plumb\" in the docs for\n\"git-ls-tree\" but came up empty. We could add just a simple \"Part of\nGit plumbing\" to a slightly longer\n\n Part of Git plumbing. Plumbing commands are more stable and are\n designed to work reliably in scripting environments ...\n\nand similarly for all the porecelain commands.\n"}]}