{"thread":{"id":"60550","subject":"[PATCH 0/4] Sparse checkout completion fixes","startedAt":"2023-11-23T17:44:11Z","lastAt":"2023-12-03T13:15:11Z","messageCount":22,"participants":["Elijah Newren via GitGitGadget","Junio C Hamano","Elijah Newren","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"485068","messageId":"pull.1349.git.1700761448.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":null,"subject":"[PATCH 0/4] Sparse checkout completion fixes","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-23T17:44:04Z","receivedAt":"2023-11-23T17:44:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This fixes a few issues with tab completion for the sparse-checkout command,\nspecifically with the \"add\" and \"set\" subcommands.\n\nThe 4th patch should probably be considered RFC; it is notable in that we\npreviously discussed a much different proposal and the general problem\narea[1], though our discussion was from a limited vantage point and none of\nus (myself included) were aware of the full context at the time. In that\nthread, Junio gave some helpful general high-level guidelines for\ncompletion. I believe the existing completion rules fail Junio's guidelines\npretty badly and that we thus need to do something else. See the lengthy\ncommit message. I implement a simple though somewhat suboptimal choice for\nthat something else (while arguing that it's at least much better than our\ncurrent solution), while also documenting with a code comment a much more\ninvolved alternative solution that we could consider in the future. Comments\non this choice welcome.\n\n[1] https://lore.kernel.org/git/xmqqv8yjz5us.fsf@gitster.g/\n\nElijah Newren (4):\n completion: squelch stray errors in sparse-checkout completion\n completion: fix logic for determining whether cone mode is active\n completion: avoid misleading completions in cone mode\n completion: avoid user confusion in non-cone mode\n\n contrib/completion/git-completion.bash | 95 +++++++++++++++++++++++++-\n 1 file changed, 92 insertions(+), 3 deletions(-)\n\n\nbase-commit: 79f2338b3746d23454308648b2491e5beba4beff\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1349%2Fnewren%2Fsparse-checkout-completion-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1349/newren/sparse-checkout-completion-fixes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1349\n-- \ngitgitgadget\n"},{"id":"485069","messageId":"591c7b8d73b1a93feaa749d68156a198a7e32a9c.1700761448.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.git.1700761448.gitgitgadget@gmail.com","subject":"[PATCH 1/4] completion: squelch stray errors in sparse-checkout completion","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-23T17:44:05Z","receivedAt":"2023-11-23T17:44:12Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIf, in the root of a project, one types\n\n git sparse-checkout set --cone ../\n\nthen an error message of the form\n\n fatal: ../: '../' is outside repository at '/home/newren/floss/git'\n\nis written to stderr, which munges the users view of their own command.\nSquelch such messages.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ba5c395d2d8..6fced40d04c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3014,7 +3014,7 @@ __gitcomp_directories ()\n \t\t\tCOMPREPLY+=(\"$c/\")\n \t\t\t_found=1\n \t\tfi\n-\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)\n+\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir 2>/dev/null)\n \n \tif [[ $_found == 0 ]] && [[ \"$cur\" =~ /$ ]]; then\n \t\t# No possible further completions any deeper, so assume we're at\n-- \ngitgitgadget\n\n"},{"id":"485070","messageId":"a15fb054579b13c6ee949ca1506d3e380da06d55.1700761448.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.git.1700761448.gitgitgadget@gmail.com","subject":"[PATCH 2/4] completion: fix logic for determining whether cone mode is active","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-23T17:44:06Z","receivedAt":"2023-11-23T17:44:12Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n_git_sparse_checkout() was checking whether we were in cone mode by\nchecking whether either:\n\n A) core.sparseCheckoutCone was \"true\"\n B) \"--cone\" was specified on the command line\n\nThis code has 2 bugs I didn't catch in my review at the time\n\n 1) core.sparseCheckout must be \"true\" for core.sparseCheckoutCone to\n be relevant (which matters since \"git sparse-checkout disable\"\n only unsets core.sparseCheckout, not core.sparseCheckoutCone)\n 2) The presence of \"--no-cone\" should override any config setting\n\nFurther, I forgot to update this logic as part of 2d95707a02\n(\"sparse-checkout: make --cone the default\", 2022-04-22) for the new\ndefault.\n\nUpdate the code for the new default and make it be more careful in\ndetermining whether to complete based on cone mode or non-cone mode.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6fced40d04c..42e9e0cba8f 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3027,6 +3027,7 @@ _git_sparse_checkout ()\n {\n \tlocal subcommands=\"list init set disable add reapply\"\n \tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n+\tlocal using_cone=true\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \t\treturn\n@@ -3037,8 +3038,15 @@ _git_sparse_checkout ()\n \t\t__gitcomp_builtin sparse-checkout_$subcommand \"\" \"--\"\n \t\t;;\n \tset,*|add,*)\n-\t\tif [ \"$(__git config core.sparseCheckoutCone)\" == \"true\" ] ||\n-\t\t[ -n \"$(__git_find_on_cmdline --cone)\" ]; then\n+\t\tif [[ \"$(__git config core.sparseCheckout)\" == \"true\" &&\n+\t\t \"$(__git config core.sparseCheckoutCone)\" == \"false\" &&\n+\t\t -z \"$(__git_find_on_cmdline --cone)\" ]]; then\n+\t\t\tusing_cone=false\n+\t\tfi\n+\t\tif [[ -n \"$(__git_find_on_cmdline --no-cone)\" ]]; then\n+\t\t\tusing_cone=false\n+\t\tfi\n+\t\tif [[ \"$using_cone\" == \"true\" ]]; then\n \t\t\t__gitcomp_directories\n \t\tfi\n \tesac\n-- \ngitgitgadget\n\n"},{"id":"485071","messageId":"e8cc5c54e60c3729e133fc68dea4ec8686fdbc63.1700761448.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.git.1700761448.gitgitgadget@gmail.com","subject":"[PATCH 3/4] completion: avoid misleading completions in cone mode","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-23T17:44:07Z","receivedAt":"2023-11-23T17:44:13Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe \"set\" and \"add\" subcommands of \"sparse-checkout\", when in cone mode,\nshould only complete on directories. For bash_completion in general,\nwhen no completions are returned for any subcommands, it will often fall\nback to standard completion of files and directories as a substitute.\nThat is not helpful here. Since we have already looked for all valid\ncompletions, if none are found then falling back to standard bash file\nand directory completion is at best actively misleading. In fact, there\nare three different ways it can be actively misleading. Add a long\ncomment in the code about how that fallback behavior can deceive, and\ndisable the fallback by returning a fake result as the sole completion.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 42e9e0cba8f..136faeca1e9 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3020,6 +3020,26 @@ __gitcomp_directories ()\n \t\t# No possible further completions any deeper, so assume we're at\n \t\t# a leaf directory and just consider it complete\n \t\t__gitcomp_direct_append \"$cur \"\n+\telif [[ $_found == 0 ]]; then\n+\t\t# No possible completions found. Avoid falling back to\n+\t\t# bash's default file and directory completion, because all\n+\t\t# valid completions have already been searched and the\n+\t\t# fallbacks can do nothing but mislead. In fact, they can\n+\t\t# mislead in three different ways:\n+\t\t# 1) Fallback file completion makes no sense when asking\n+\t\t# for directory completions, as this function does.\n+\t\t# 2) Fallback directory completion is bad because\n+\t\t# e.g. \"/pro\" is invalid and should NOT complete to\n+\t\t# \"/proc\".\n+\t\t# 3) Fallback file/directory completion only completes\n+\t\t# on paths that exist in the current working tree,\n+\t\t# i.e. which are *already* part of their\n+\t\t# sparse-checkout. Thus, normal file and directory\n+\t\t# completion is always useless for \"git\n+\t\t# sparse-checkout add\" and is also probelmatic for\n+\t\t# \"git sparse-checkout set\" unless using it to\n+\t\t# strictly narrow the checkout.\n+\t\tCOMPREPLY=( \"\" )\n \tfi\n }\n \n-- \ngitgitgadget\n\n"},{"id":"485072","messageId":"fe8669a3f4f05c186e497f870c7e7ba9a94ac63f.1700761448.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.git.1700761448.gitgitgadget@gmail.com","subject":"[PATCH 4/4] completion: avoid user confusion in non-cone mode","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-23T17:44:08Z","receivedAt":"2023-11-23T17:44:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIt is tempting to think of \"files and directories\" of the current\ndirectory as valid inputs to the add and set subcommands of git\nsparse-checkout. However, in non-cone mode, they often aren't and using\nthem as potential completions leads to *many* forms of confusion:\n\nIssue #1. It provides the *wrong* files and directories.\n\nFor\n git sparse-checkout add\nwe always want to add files and directories not currently in our sparse\ncheckout, which means we want file and directories not currently present\nin the current working tree. Providing the files and directories\ncurrently present is thus always wrong.\n\nFor\n git sparse-checkout set\nwe have a similar problem except in the subset of cases where we are\ntrying to narrow our checkout to a strict subset of what we already\nhave. That is not a very common scenario, especially since it often\ndoes not even happen to be true for the first use of the command; for\nyears we required users to create a sparse-checkout via\n git sparse-checkout init\n git sparse-checkout set \n(or use a clone option that did the init step for you at clone time).\nThe init command creates a minimal sparse-checkout with just the\ntop-level directory present, meaning the set command has to be used to\nexpand the checkout. Thus, only in a special and perhaps unusual cases\nwould any of the suggestions from normal file and directory completion\nbe appropriate.\n\nIssue #2: Suggesting patterns that lead to warnings is unfriendly.\n\nIf the user specifies any regular file and omits the leading '/', then\nthe sparse-checkout command will warn the user that their command is\nproblematic and suggest they use a leading slash instead.\n\nIssue #3: Completion gets confused by leading '/', and provides wrong paths.\n\nUsers often want to anchor their patterns to the toplevel of the\nrepository, especially when listing individual files. There are a\nnumber of reasons for this, but notably even sparse-checkout encourages\nthem to do so (as noted above). However, if users do so (via adding a\nleading '/' to their pattern), then bash completion will interpret the\nleading slash not as a request for a path at the toplevel of the\nrepository, but as a request for a path at the root of the filesytem.\nThat means at best that completion cannot help with such paths, and if\nit does find any completions, they are almost guaranteed to be wrong.\n\nIssue #4: Suggesting invalid patterns from subdirectories is unfriendly.\n\nThere is no per-directory equivalent to .gitignore with\nsparse-checkouts. There is only a single worktree-global\n$GIT_DIR/info/sparse-checkout file. As such, paths to files must be\nspecified relative to the toplevel of a repository. Providing\nsuggestions of paths that are relative to the current working directory,\nas bash completion defaults to, is wrong when the current working\ndirectory is not the worktree toplevel directory.\n\nIssue #5: Paths with special characters will be interpreted incorrectly\n\nThe entries in the sparse-checkout file are patterns, not paths. While\nmost paths also qualify as patterns (though even in such cases it would\nbe better for users to not use them directly but prefix them with a\nleading '/'), there are a variety of special characters that would need\nspecial escaping beyond the normal shell escaping: '*', '?', '\\', '[',\n']', and any leading '#' or '!'. If completion suggests any such paths,\nusers will likely expect them to be treated as an exact path rather than\nas a pattern that might match some number of files other than 1.\n\nBecause of the combination of the above issues, turn completion off for\nthe `set` and `add` subcommands of `sparse-checkout` when in non-cone\nmode, but leave a NEEDSWORK comment specifying what could theoretically\nbe done if someone wanted to provide completion rules that were more\nhelpful than harmful.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 61 ++++++++++++++++++++++++++\n 1 file changed, 61 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 136faeca1e9..7d460da2fab 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3068,6 +3068,67 @@ _git_sparse_checkout ()\n \t\tfi\n \t\tif [[ \"$using_cone\" == \"true\" ]]; then\n \t\t\t__gitcomp_directories\n+\t\telse\n+\t\t\t# NEEDSWORK: It might be useful to provide a\n+\t\t\t# completion function which:\n+\t\t\t#\n+\t\t\t# 1. Provides completions based on\n+\t\t\t# files/directories that exist in HEAD, not\n+\t\t\t# just those currently present in the working\n+\t\t\t# tree. Bash's default file and directory\n+\t\t\t# completion is totally useless for \"git\n+\t\t\t# sparse-checkout add\" because of this. It is\n+\t\t\t# likewise problematic for \"git\n+\t\t\t# sparse-checkout set\" except in those subset\n+\t\t\t# of cases when trying to narrow scope to a\n+\t\t\t# strict subset of what you already have\n+\t\t\t# checked out.\n+\t\t\t#\n+\t\t\t# 2. Always provides file/directory completions\n+\t\t\t# with a prepended leading '/', so that\n+\t\t\t# files/directories are only searched at the\n+\t\t\t# relevant level rather than throughout all\n+\t\t\t# trees in the hierarchy. Doing this also\n+\t\t\t# avoids suggesting the user run a\n+\t\t\t# sparse-checkout command that will result in\n+\t\t\t# a warning be thrown at the user.\n+\t\t\t#\n+\t\t\t# 3. Does not accidentally search the root of\n+\t\t\t# the filesystem when a path with a leading\n+\t\t\t# slash is specified. (\"git sparse-checkout\n+\t\t\t# add /ho\" should not complete to\n+\t\t\t# \"/home\" but to e.g. \"/hooks\" if there is a\n+\t\t\t# \"hooks\" in the top of the repository.)\n+\t\t\t#\n+\t\t\t# 4. Provides no completions when run from a\n+\t\t\t# subdirectory of the repository root. (If we\n+\t\t\t# did provide file/directory completions, the\n+\t\t\t# user would just get a \"please run from the\n+\t\t\t# toplevel directory\" error message when they\n+\t\t\t# ran it. *Further*, if the user did rerun\n+\t\t\t# the command from the toplevel, the\n+\t\t\t# completions we previously provided would\n+\t\t\t# likely be wrong as they'd be relative to the\n+\t\t\t# subdirectory rather than the repository\n+\t\t\t# root. That could lead to users getting a\n+\t\t\t# nasty surprise based on trying to use a\n+\t\t\t# command we helped them create.)\n+\t\t\t#\n+\t\t\t# 5. Provides escaped completions for any paths\n+\t\t\t# containing a '*', '?', '\\', '[', ']', or\n+\t\t\t# leading '#' or '!'. (These characters might\n+\t\t\t# already be escaped to protect from the\n+\t\t\t# shell, but they need an *extra* layer of\n+\t\t\t# escaping to prevent the pattern parsing in\n+\t\t\t# Git from seeing them as special characters.)\n+\t\t\t#\n+\t\t\t# Of course, this would be a lot of work, so for now,\n+\t\t\t# just avoid the many forms of user confusion that\n+\t\t\t# could be caused by providing bad completions by\n+\t\t\t# providing a fake completion to avoid falling back to\n+\t\t\t# bash's normal file and directory completion.\n+\n+\t\t\tCOMPREPLY=( \"\" )\n \t\tfi\n \tesac\n }\n-- \ngitgitgadget\n"},{"id":"485082","messageId":"xmqqo7fk9cdt.fsf@gitster.g","threadId":"60550","inReplyTo":"fe8669a3f4f05c186e497f870c7e7ba9a94ac63f.1700761448.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-24T01:19:10Z","receivedAt":"2023-11-24T01:19:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> \t\tif [[ \"$using_cone\" == \"true\" ]]; then\n> \t\t\t__gitcomp_directories\n\nHmph, doesn't \"Providing the files and directories currently present\nis thus always wrong.\" apply equally to cone mode?\n\n> +\t\telse\n> +\t\t\t# NEEDSWORK: It might be useful to provide a\n> +\t\t\t# completion function which:\n> +\t\t\t#\n> +\t\t\t# 1. Provides completions based on\n> +\t\t\t# files/directories that exist in HEAD, not\n> +\t\t\t# just those currently present in the working\n> +\t\t\t# tree.\n\nThis makes a lot of sense. May make even more sense with\ns/HEAD/index/, though.\n\n> +\t\t\t# 4. Provides no completions when run from a\n> +\t\t\t# subdirectory of the repository root. (If we\n> +\t\t\t# did provide file/directory completions, the\n> +\t\t\t# user would just get a \"please run from the\n> +\t\t\t# toplevel directory\" error message when they\n> +\t\t\t# ran it. *Further*, if the user did rerun\n> +\t\t\t# the command from the toplevel, the\n> +\t\t\t# completions we previously provided would\n> +\t\t\t# likely be wrong as they'd be relative to the\n> +\t\t\t# subdirectory rather than the repository\n> +\t\t\t# root. That could lead to users getting a\n> +\t\t\t# nasty surprise based on trying to use a\n> +\t\t\t# command we helped them create.)\n\nHmph, would an obvious alternative to (1) check against the HEAD (or\nthe index) to see if the prefix string matches an entity at the\ncurrent directory level, and then (2) to prefix the result of the\nprevious step with \"/$(git rev-parse --show-prefix)\" work? That is\nsomething like this:\n\n $ cd t\n $ git sparse-checkout add help\n ->\n $ git sparse-checkout add /t/helper/\n\nand when the user gave the full path from the root level, do the\nobvious:\n\n $ cd t\n $ git sparse-checkout add /t/help\n ->\n $ git sparse-checkout add /t/helper/\n\nAnother more fundamental approach to avoid \"confusion\" this bullet\nitem tries to side step might be to *fix* the command that gets\ncompleted. As \"git sparse-checkout --help\" is marked as\nEXPERIMENTAL in capital letters, we should be able to say \"what was\ntraditionally known as 'add' is from now on called 'add-pattern' and\ncommand line completion would not get in the way; the 'add'\nsubcommand now takes only literal paths, not patterns, that are\nrelative to the current directory\" if we wanted to.\n\n> +\t\t\t# 5. Provides escaped completions for any paths\n> +\t\t\t# containing a '*', '?', '\\', '[', ']', or\n> +\t\t\t# leading '#' or '!'. (These characters might\n> +\t\t\t# already be escaped to protect from the\n> +\t\t\t# shell, but they need an *extra* layer of\n> +\t\t\t# escaping to prevent the pattern parsing in\n> +\t\t\t# Git from seeing them as special characters.)\n> +\t\t\t#\n> +\t\t\t# Of course, this would be a lot of work, so for now,\n> +\t\t\t# just avoid the many forms of user confusion that\n> +\t\t\t# could be caused by providing bad completions by\n> +\t\t\t# providing a fake completion to avoid falling back to\n> +\t\t\t# bash's normal file and directory completion.\n\n> +\t\t\tCOMPREPLY=( \"\" )\n> \t\tfi\n> \tesac\n> }\n"},{"id":"485125","messageId":"CABPp-BE73cDq-DBKzsw3R0awiKn5J5LCfdXRULHyPbrS9GEn4Q@mail.gmail.com","threadId":"60550","inReplyTo":"xmqqo7fk9cdt.fsf@gitster.g","subject":"Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-11-24T15:28:07Z","receivedAt":"2023-11-24T15:28:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Nov 23, 2023 at 5:19 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > if [[ \"$using_cone\" == \"true\" ]]; then\n> > __gitcomp_directories\n>\n> Hmph, doesn't \"Providing the files and directories currently present\n> is thus always wrong.\" apply equally to cone mode?\n\nAbsolutely, it definitely applies to cone mode. We (mostly) fixed\nthat a long time ago, making it not complete on the files &\ndirectories currently present. In particular, the\n__gitcomp_directories() function highlighted here completes on the\noutput of `git ls-tree -z -d --name-only HEAD`.\n\nHowever, before this series, there was a problem when\n__gitcomp_directories() finds no possible completions. In that case,\nthe code would fall back to bash-completion's default of completing on\nall files and directories currently present. But that was fixed in\npatch 3 of this series to avoid that fallback.\n\nThis patch, though, isn't about cone mode. It's about fixing (or at\nleast improving) non-cone mode.\n\n> > + else\n> > + # NEEDSWORK: It might be useful to provide a\n> > + # completion function which:\n> > + #\n> > + # 1. Provides completions based on\n> > + # files/directories that exist in HEAD, not\n> > + # just those currently present in the working\n> > + # tree.\n>\n> This makes a lot of sense. May make even more sense with\n> s/HEAD/index/, though.\n\nOoh, interesting. That wouldn't work with the sparse index (where\npaths we want to complete on are currently missing from the index\ntoo), but sparse index is restricted to cone mode, and we're\ndiscussing non-cone-mode here. So, this might be a basis for a good\nalternative.\n\n> > + # 4. Provides no completions when run from a\n> > + # subdirectory of the repository root. (If we\n> > + # did provide file/directory completions, the\n> > + # user would just get a \"please run from the\n> > + # toplevel directory\" error message when they\n> > + # ran it. *Further*, if the user did rerun\n> > + # the command from the toplevel, the\n> > + # completions we previously provided would\n> > + # likely be wrong as they'd be relative to the\n> > + # subdirectory rather than the repository\n> > + # root. That could lead to users getting a\n> > + # nasty surprise based on trying to use a\n> > + # command we helped them create.)\n>\n> Hmph, would an obvious alternative to (1) check against the HEAD (or\n> the index) to see if the prefix string matches an entity at the\n> current directory level, and then (2) to prefix the result of the\n> previous step with \"/$(git rev-parse --show-prefix)\" work? That is\n> something like this:\n>\n> $ cd t\n> $ git sparse-checkout add help\n> ->\n> $ git sparse-checkout add /t/helper/\n\nI thought bash-completion was only for completions, not for startings\nas well. Was I mistaken?\n\n> and when the user gave the full path from the root level, do the\n> obvious:\n>\n> $ cd t\n> $ git sparse-checkout add /t/help\n> ->\n> $ git sparse-checkout add /t/helper/\n>\n> Another more fundamental approach to avoid \"confusion\" this bullet\n> item tries to side step might be to *fix* the command that gets\n> completed. As \"git sparse-checkout --help\" is marked as\n> EXPERIMENTAL in capital letters, we should be able to say \"what was\n> traditionally known as 'add' is from now on called 'add-pattern' and\n> command line completion would not get in the way; the 'add'\n> subcommand now takes only literal paths, not patterns, that are\n> relative to the current directory\" if we wanted to.\n\nThat's interesting...but it opens up a new can of worms:\n * Would we also need both `set-patterns` and `set`, in addition to\n`add-patterns` and `add`?\n * In cone mode, the paths passed are literal directories (and only\ndirectories; no individual files), but the thing added is a\ntelescoping \"cone\" of leading directories as well. Does this make it\npotentially confusing to users to say that `add` only takes literal\npaths?\n * In cone mode (the default), should `add-patterns` just be an\nerror, since no pattern specification is allowed?\n * In the git-sparse-checkout manual, for performance reasons, we\nrecommend users _not_ specify individual paths in non-cone mode.\nWould our recommendation then be to just not use `add` or `set` and\nonly use `add-patterns` and `set-patterns`? If so, what have we\naccomplished by adding the new names?\n\nMaybe I'm missing something about your suggestion, but this seems much\nmore complex than the simple solution we implemented in bb8b5e9a90d\n(\"sparse-checkout: pay attention to prefix for {set, add}\",\n2022-02-19) for the !core_sparse_checkout_cone case. I like the\nsimple solution there, though that simple solution omitted modifying\nthe completion rules in a way that was consistent (i.e. that returns\nnothing when the user is running from a subdirectory).\n\n\n> > + # 5. Provides escaped completions for any paths\n> > + # containing a '*', '?', '\\', '[', ']', or\n> > + # leading '#' or '!'. (These characters might\n> > + # already be escaped to protect from the\n> > + # shell, but they need an *extra* layer of\n> > + # escaping to prevent the pattern parsing in\n> > + # Git from seeing them as special characters.)\n> > + #\n> > + # Of course, this would be a lot of work, so for now,\n> > + # just avoid the many forms of user confusion that\n> > + # could be caused by providing bad completions by\n> > + # providing a fake completion to avoid falling back to\n> > + # bash's normal file and directory completion.\n>\n> > + COMPREPLY=( \"\" )\n> > fi\n> > esac\n> > }\n"},{"id":"485135","messageId":"20231124183938.GB11157@szeder.dev","threadId":"60550","inReplyTo":"591c7b8d73b1a93feaa749d68156a198a7e32a9c.1700761448.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/4] completion: squelch stray errors in sparse-checkout completion","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2023-11-24T18:39:38Z","receivedAt":"2023-11-24T18:39:42Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Nov 23, 2023 at 05:44:05PM +0000, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> If, in the root of a project, one types\n> \n> git sparse-checkout set --cone ../\n> \n> then an error message of the form\n> \n> fatal: ../: '../' is outside repository at '/home/newren/floss/git'\n> \n> is written to stderr, which munges the users view of their own command.\n> Squelch such messages.\n> \n> Signed-off-by: Elijah Newren \n> ---\n> contrib/completion/git-completion.bash | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index ba5c395d2d8..6fced40d04c 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -3014,7 +3014,7 @@ __gitcomp_directories ()\n> \t\t\tCOMPREPLY+=(\"$c/\")\n> \t\t\t_found=1\n> \t\tfi\n> -\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)\n> +\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir 2>/dev/null)\n\nIt would be better to use the __git wrapper instead, like the wast\nmajority of git invocations in our completion script, because it not\nonly takes care of squelching standard error, but also takes into\naccount any -C dir and/or --git-dir options present on the command\nline.\n\ne15098a314 (completion: consolidate silencing errors from git\ncommands, 2017-02-03)\n\n\n> \n> \tif [[ $_found == 0 ]] && [[ \"$cur\" =~ /$ ]]; then\n> \t\t# No possible further completions any deeper, so assume we're at\n> -- \n> gitgitgadget\n> \n> \n"},{"id":"485136","messageId":"CABPp-BGn0YT4e_ScWVR5riiuZ5kDd7X41WV-xMe91BGeuQ5xPQ@mail.gmail.com","threadId":"60550","inReplyTo":"20231124183938.GB11157@szeder.dev","subject":"Re: [PATCH 1/4] completion: squelch stray errors in sparse-checkout completion","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-11-24T20:05:02Z","receivedAt":"2023-11-24T20:05:16Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Nov 24, 2023 at 10:39 AM SZEDER Gábor wrote:\n>\n> On Thu, Nov 23, 2023 at 05:44:05PM +0000, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren \n> >\n> > If, in the root of a project, one types\n> >\n> > git sparse-checkout set --cone ../\n> >\n> > then an error message of the form\n> >\n> > fatal: ../: '../' is outside repository at '/home/newren/floss/git'\n> >\n> > is written to stderr, which munges the users view of their own command.\n> > Squelch such messages.\n> >\n> > Signed-off-by: Elijah Newren \n> > ---\n> > contrib/completion/git-completion.bash | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index ba5c395d2d8..6fced40d04c 100644\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -3014,7 +3014,7 @@ __gitcomp_directories ()\n> > COMPREPLY+=(\"$c/\")\n> > _found=1\n> > fi\n> > - done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)\n> > + done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir 2>/dev/null)\n>\n> It would be better to use the __git wrapper instead, like the wast\n> majority of git invocations in our completion script, because it not\n> only takes care of squelching standard error, but also takes into\n> account any -C dir and/or --git-dir options present on the command\n> line.\n>\n> e15098a314 (completion: consolidate silencing errors from git\n> commands, 2017-02-03)\n\nOoh, nice! Thanks for the pointer, I was unaware. I'll make that change.\n"},{"id":"485151","messageId":"97e20e3b99d8261815450e8a7c137938caaf5a6e.1700985086.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v2.git.1700985086.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] completion: squelch stray errors in sparse-checkout completion","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-26T07:51:23Z","receivedAt":"2023-11-26T07:51:30Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIf, in the root of a project, one types\n\n git sparse-checkout set --cone ../\n\nthen an error message of the form\n\n fatal: ../: '../' is outside repository at '/home/newren/floss/git'\n\nis written to stderr, which munges the users view of their own command.\nSquelch such messages by using the __git() wrapper, designed for this\npurpose; see commit e15098a314 (completion: consolidate silencing errors\nfrom git commands, 2017-02-03) for more on the wrapper.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 13a39ebd2e7..b8661701718 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3084,7 +3084,7 @@ __gitcomp_directories ()\n \t\t\tCOMPREPLY+=(\"$c/\")\n \t\t\t_found=1\n \t\tfi\n-\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)\n+\tdone < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)\n \n \tif [[ $_found == 0 ]] && [[ \"$cur\" =~ /$ ]]; then\n \t\t# No possible further completions any deeper, so assume we're at\n-- \ngitgitgadget\n\n"},{"id":"485152","messageId":"pull.1349.v2.git.1700985086.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.git.1700761448.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] Sparse checkout completion fixes","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-26T07:51:22Z","receivedAt":"2023-11-26T07:51:30Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This fixes a few issues with tab completion for the sparse-checkout command,\nspecifically with the \"add\" and \"set\" subcommands.\n\nAs noted in v1, the 4th patch implements a somewhat suboptimal solution that\nat least improves the situation, while also documenting with a code comment\na much more involved alternative solution that we could consider in the\nfuture.\n\nChanges since v1:\n\n * Use __git wrapper function to squelch errors, as suggested by SZEDER\n Gábor\n * note that we could use the index or HEAD for the more involved solution\n in patch 4.\n\n[1] https://lore.kernel.org/git/xmqqv8yjz5us.fsf@gitster.g/\n\nElijah Newren (4):\n completion: squelch stray errors in sparse-checkout completion\n completion: fix logic for determining whether cone mode is active\n completion: avoid misleading completions in cone mode\n completion: avoid user confusion in non-cone mode\n\n contrib/completion/git-completion.bash | 96 +++++++++++++++++++++++++-\n 1 file changed, 93 insertions(+), 3 deletions(-)\n\n\nbase-commit: 564d0252ca632e0264ed670534a51d18a689ef5d\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1349%2Fnewren%2Fsparse-checkout-completion-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1349/newren/sparse-checkout-completion-fixes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1349\n\nRange-diff vs v1:\n\n 1: 591c7b8d73b ! 1: 97e20e3b99d completion: squelch stray errors in sparse-checkout completion\n @@ Commit message\n fatal: ../: '../' is outside repository at '/home/newren/floss/git'\n \n is written to stderr, which munges the users view of their own command.\n - Squelch such messages.\n + Squelch such messages by using the __git() wrapper, designed for this\n + purpose; see commit e15098a314 (completion: consolidate silencing errors\n + from git commands, 2017-02-03) for more on the wrapper.\n \n Signed-off-by: Elijah Newren \n \n @@ contrib/completion/git-completion.bash: __gitcomp_directories ()\n \t\t\t_found=1\n \t\tfi\n -\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)\n -+\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir 2>/dev/null)\n ++\tdone < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)\n \n \tif [[ $_found == 0 ]] && [[ \"$cur\" =~ /$ ]]; then\n \t\t# No possible further completions any deeper, so assume we're at\n 2: a15fb054579 = 2: 212ba35ed46 completion: fix logic for determining whether cone mode is active\n 3: e8cc5c54e60 = 3: 1cbbcd9097c completion: avoid misleading completions in cone mode\n 4: fe8669a3f4f ! 4: 604f21dc827 completion: avoid user confusion in non-cone mode\n @@ contrib/completion/git-completion.bash: _git_sparse_checkout ()\n +\t\t\t# completion function which:\n +\t\t\t#\n +\t\t\t# 1. Provides completions based on\n -+\t\t\t# files/directories that exist in HEAD, not\n -+\t\t\t# just those currently present in the working\n -+\t\t\t# tree. Bash's default file and directory\n -+\t\t\t# completion is totally useless for \"git\n -+\t\t\t# sparse-checkout add\" because of this. It is\n -+\t\t\t# likewise problematic for \"git\n -+\t\t\t# sparse-checkout set\" except in those subset\n -+\t\t\t# of cases when trying to narrow scope to a\n -+\t\t\t# strict subset of what you already have\n -+\t\t\t# checked out.\n ++\t\t\t# files/directories that exist in HEAD (or in\n ++\t\t\t# the index since sparse-index isn't possible\n ++\t\t\t# in non-cone mode), not just those currently\n ++\t\t\t# present in the working tree. Bash's\n ++\t\t\t# default file and directory completion is\n ++\t\t\t# totally useless for \"git sparse-checkout\n ++\t\t\t# add\" because of this. It is likewise\n ++\t\t\t# problematic for \"git sparse-checkout set\"\n ++\t\t\t# except in those subset of cases when trying\n ++\t\t\t# to narrow scope to a strict subset of what\n ++\t\t\t# you already have checked out.\n +\t\t\t#\n +\t\t\t# 2. Always provides file/directory completions\n +\t\t\t# with a prepended leading '/', so that\n\n-- \ngitgitgadget\n"},{"id":"485153","messageId":"212ba35ed469b63fc75c51b61588025c303a162d.1700985086.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v2.git.1700985086.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] completion: fix logic for determining whether cone mode is active","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-26T07:51:24Z","receivedAt":"2023-11-26T07:51:31Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n_git_sparse_checkout() was checking whether we were in cone mode by\nchecking whether either:\n\n A) core.sparseCheckoutCone was \"true\"\n B) \"--cone\" was specified on the command line\n\nThis code has 2 bugs I didn't catch in my review at the time\n\n 1) core.sparseCheckout must be \"true\" for core.sparseCheckoutCone to\n be relevant (which matters since \"git sparse-checkout disable\"\n only unsets core.sparseCheckout, not core.sparseCheckoutCone)\n 2) The presence of \"--no-cone\" should override any config setting\n\nFurther, I forgot to update this logic as part of 2d95707a02\n(\"sparse-checkout: make --cone the default\", 2022-04-22) for the new\ndefault.\n\nUpdate the code for the new default and make it be more careful in\ndetermining whether to complete based on cone mode or non-cone mode.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b8661701718..7aa66c19ede 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3097,6 +3097,7 @@ _git_sparse_checkout ()\n {\n \tlocal subcommands=\"list init set disable add reapply\"\n \tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n+\tlocal using_cone=true\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \t\treturn\n@@ -3107,8 +3108,15 @@ _git_sparse_checkout ()\n \t\t__gitcomp_builtin sparse-checkout_$subcommand \"\" \"--\"\n \t\t;;\n \tset,*|add,*)\n-\t\tif [ \"$(__git config core.sparseCheckoutCone)\" == \"true\" ] ||\n-\t\t[ -n \"$(__git_find_on_cmdline --cone)\" ]; then\n+\t\tif [[ \"$(__git config core.sparseCheckout)\" == \"true\" &&\n+\t\t \"$(__git config core.sparseCheckoutCone)\" == \"false\" &&\n+\t\t -z \"$(__git_find_on_cmdline --cone)\" ]]; then\n+\t\t\tusing_cone=false\n+\t\tfi\n+\t\tif [[ -n \"$(__git_find_on_cmdline --no-cone)\" ]]; then\n+\t\t\tusing_cone=false\n+\t\tfi\n+\t\tif [[ \"$using_cone\" == \"true\" ]]; then\n \t\t\t__gitcomp_directories\n \t\tfi\n \tesac\n-- \ngitgitgadget\n\n"},{"id":"485154","messageId":"1cbbcd9097c96b7646fb0c756c3964413f2a021f.1700985086.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v2.git.1700985086.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] completion: avoid misleading completions in cone mode","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-26T07:51:25Z","receivedAt":"2023-11-26T07:51:32Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe \"set\" and \"add\" subcommands of \"sparse-checkout\", when in cone mode,\nshould only complete on directories. For bash_completion in general,\nwhen no completions are returned for any subcommands, it will often fall\nback to standard completion of files and directories as a substitute.\nThat is not helpful here. Since we have already looked for all valid\ncompletions, if none are found then falling back to standard bash file\nand directory completion is at best actively misleading. In fact, there\nare three different ways it can be actively misleading. Add a long\ncomment in the code about how that fallback behavior can deceive, and\ndisable the fallback by returning a fake result as the sole completion.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7aa66c19ede..c614e5d4f07 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3090,6 +3090,26 @@ __gitcomp_directories ()\n \t\t# No possible further completions any deeper, so assume we're at\n \t\t# a leaf directory and just consider it complete\n \t\t__gitcomp_direct_append \"$cur \"\n+\telif [[ $_found == 0 ]]; then\n+\t\t# No possible completions found. Avoid falling back to\n+\t\t# bash's default file and directory completion, because all\n+\t\t# valid completions have already been searched and the\n+\t\t# fallbacks can do nothing but mislead. In fact, they can\n+\t\t# mislead in three different ways:\n+\t\t# 1) Fallback file completion makes no sense when asking\n+\t\t# for directory completions, as this function does.\n+\t\t# 2) Fallback directory completion is bad because\n+\t\t# e.g. \"/pro\" is invalid and should NOT complete to\n+\t\t# \"/proc\".\n+\t\t# 3) Fallback file/directory completion only completes\n+\t\t# on paths that exist in the current working tree,\n+\t\t# i.e. which are *already* part of their\n+\t\t# sparse-checkout. Thus, normal file and directory\n+\t\t# completion is always useless for \"git\n+\t\t# sparse-checkout add\" and is also probelmatic for\n+\t\t# \"git sparse-checkout set\" unless using it to\n+\t\t# strictly narrow the checkout.\n+\t\tCOMPREPLY=( \"\" )\n \tfi\n }\n \n-- \ngitgitgadget\n\n"},{"id":"485155","messageId":"604f21dc827ab8f3378ff84e7ce7939ef82d78cd.1700985086.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v2.git.1700985086.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] completion: avoid user confusion in non-cone mode","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-26T07:51:26Z","receivedAt":"2023-11-26T07:51:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIt is tempting to think of \"files and directories\" of the current\ndirectory as valid inputs to the add and set subcommands of git\nsparse-checkout. However, in non-cone mode, they often aren't and using\nthem as potential completions leads to *many* forms of confusion:\n\nIssue #1. It provides the *wrong* files and directories.\n\nFor\n git sparse-checkout add\nwe always want to add files and directories not currently in our sparse\ncheckout, which means we want file and directories not currently present\nin the current working tree. Providing the files and directories\ncurrently present is thus always wrong.\n\nFor\n git sparse-checkout set\nwe have a similar problem except in the subset of cases where we are\ntrying to narrow our checkout to a strict subset of what we already\nhave. That is not a very common scenario, especially since it often\ndoes not even happen to be true for the first use of the command; for\nyears we required users to create a sparse-checkout via\n git sparse-checkout init\n git sparse-checkout set \n(or use a clone option that did the init step for you at clone time).\nThe init command creates a minimal sparse-checkout with just the\ntop-level directory present, meaning the set command has to be used to\nexpand the checkout. Thus, only in a special and perhaps unusual cases\nwould any of the suggestions from normal file and directory completion\nbe appropriate.\n\nIssue #2: Suggesting patterns that lead to warnings is unfriendly.\n\nIf the user specifies any regular file and omits the leading '/', then\nthe sparse-checkout command will warn the user that their command is\nproblematic and suggest they use a leading slash instead.\n\nIssue #3: Completion gets confused by leading '/', and provides wrong paths.\n\nUsers often want to anchor their patterns to the toplevel of the\nrepository, especially when listing individual files. There are a\nnumber of reasons for this, but notably even sparse-checkout encourages\nthem to do so (as noted above). However, if users do so (via adding a\nleading '/' to their pattern), then bash completion will interpret the\nleading slash not as a request for a path at the toplevel of the\nrepository, but as a request for a path at the root of the filesytem.\nThat means at best that completion cannot help with such paths, and if\nit does find any completions, they are almost guaranteed to be wrong.\n\nIssue #4: Suggesting invalid patterns from subdirectories is unfriendly.\n\nThere is no per-directory equivalent to .gitignore with\nsparse-checkouts. There is only a single worktree-global\n$GIT_DIR/info/sparse-checkout file. As such, paths to files must be\nspecified relative to the toplevel of a repository. Providing\nsuggestions of paths that are relative to the current working directory,\nas bash completion defaults to, is wrong when the current working\ndirectory is not the worktree toplevel directory.\n\nIssue #5: Paths with special characters will be interpreted incorrectly\n\nThe entries in the sparse-checkout file are patterns, not paths. While\nmost paths also qualify as patterns (though even in such cases it would\nbe better for users to not use them directly but prefix them with a\nleading '/'), there are a variety of special characters that would need\nspecial escaping beyond the normal shell escaping: '*', '?', '\\', '[',\n']', and any leading '#' or '!'. If completion suggests any such paths,\nusers will likely expect them to be treated as an exact path rather than\nas a pattern that might match some number of files other than 1.\n\nBecause of the combination of the above issues, turn completion off for\nthe `set` and `add` subcommands of `sparse-checkout` when in non-cone\nmode, but leave a NEEDSWORK comment specifying what could theoretically\nbe done if someone wanted to provide completion rules that were more\nhelpful than harmful.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 62 ++++++++++++++++++++++++++\n 1 file changed, 62 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c614e5d4f07..1e07dc8b73e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3138,6 +3138,68 @@ _git_sparse_checkout ()\n \t\tfi\n \t\tif [[ \"$using_cone\" == \"true\" ]]; then\n \t\t\t__gitcomp_directories\n+\t\telse\n+\t\t\t# NEEDSWORK: It might be useful to provide a\n+\t\t\t# completion function which:\n+\t\t\t#\n+\t\t\t# 1. Provides completions based on\n+\t\t\t# files/directories that exist in HEAD (or in\n+\t\t\t# the index since sparse-index isn't possible\n+\t\t\t# in non-cone mode), not just those currently\n+\t\t\t# present in the working tree. Bash's\n+\t\t\t# default file and directory completion is\n+\t\t\t# totally useless for \"git sparse-checkout\n+\t\t\t# add\" because of this. It is likewise\n+\t\t\t# problematic for \"git sparse-checkout set\"\n+\t\t\t# except in those subset of cases when trying\n+\t\t\t# to narrow scope to a strict subset of what\n+\t\t\t# you already have checked out.\n+\t\t\t#\n+\t\t\t# 2. Always provides file/directory completions\n+\t\t\t# with a prepended leading '/', so that\n+\t\t\t# files/directories are only searched at the\n+\t\t\t# relevant level rather than throughout all\n+\t\t\t# trees in the hierarchy. Doing this also\n+\t\t\t# avoids suggesting the user run a\n+\t\t\t# sparse-checkout command that will result in\n+\t\t\t# a warning be thrown at the user.\n+\t\t\t#\n+\t\t\t# 3. Does not accidentally search the root of\n+\t\t\t# the filesystem when a path with a leading\n+\t\t\t# slash is specified. (\"git sparse-checkout\n+\t\t\t# add /ho\" should not complete to\n+\t\t\t# \"/home\" but to e.g. \"/hooks\" if there is a\n+\t\t\t# \"hooks\" in the top of the repository.)\n+\t\t\t#\n+\t\t\t# 4. Provides no completions when run from a\n+\t\t\t# subdirectory of the repository root. (If we\n+\t\t\t# did provide file/directory completions, the\n+\t\t\t# user would just get a \"please run from the\n+\t\t\t# toplevel directory\" error message when they\n+\t\t\t# ran it. *Further*, if the user did rerun\n+\t\t\t# the command from the toplevel, the\n+\t\t\t# completions we previously provided would\n+\t\t\t# likely be wrong as they'd be relative to the\n+\t\t\t# subdirectory rather than the repository\n+\t\t\t# root. That could lead to users getting a\n+\t\t\t# nasty surprise based on trying to use a\n+\t\t\t# command we helped them create.)\n+\t\t\t#\n+\t\t\t# 5. Provides escaped completions for any paths\n+\t\t\t# containing a '*', '?', '\\', '[', ']', or\n+\t\t\t# leading '#' or '!'. (These characters might\n+\t\t\t# already be escaped to protect from the\n+\t\t\t# shell, but they need an *extra* layer of\n+\t\t\t# escaping to prevent the pattern parsing in\n+\t\t\t# Git from seeing them as special characters.)\n+\t\t\t#\n+\t\t\t# Of course, this would be a lot of work, so for now,\n+\t\t\t# just avoid the many forms of user confusion that\n+\t\t\t# could be caused by providing bad completions by\n+\t\t\t# providing a fake completion to avoid falling back to\n+\t\t\t# bash's normal file and directory completion.\n+\n+\t\t\tCOMPREPLY=( \"\" )\n \t\tfi\n \tesac\n }\n-- \ngitgitgadget\n"},{"id":"485163","messageId":"xmqq34ws3rfa.fsf@gitster.g","threadId":"60550","inReplyTo":"CABPp-BE73cDq-DBKzsw3R0awiKn5J5LCfdXRULHyPbrS9GEn4Q@mail.gmail.com","subject":"Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-27T01:39:53Z","receivedAt":"2023-11-27T01:39:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> > if [[ \"$using_cone\" == \"true\" ]]; then\n>> > __gitcomp_directories\n>>\n>> Hmph, doesn't \"Providing the files and directories currently present\n>> is thus always wrong.\" apply equally to cone mode?\n>\n> Absolutely, it definitely applies to cone mode. We (mostly) fixed\n> that a long time ago, making it not complete on the files &\n> directories currently present. In particular, the\n> __gitcomp_directories() function highlighted here completes on the\n> output of `git ls-tree -z -d --name-only HEAD`.\n\nThanks; what I missed was exactly what __gitcomp_directories does\nnot do you explained above.\n\n>> > + # 4. Provides no completions when run from a\n>> > + # subdirectory of the repository root. (If we\n>> > + # did provide file/directory completions, the\n>> > + # user would just get a \"please run from the\n>> > + # toplevel directory\" error message when they\n>> > + # ran it. *Further*, if the user did rerun\n>> > + # the command from the toplevel, the\n>> > + # completions we previously provided would\n>> > + # likely be wrong as they'd be relative to the\n>> > + # subdirectory rather than the repository\n>> > + # root. That could lead to users getting a\n>> > + # nasty surprise based on trying to use a\n>> > + # command we helped them create.)\n>>\n>> Hmph, would an obvious alternative to (1) check against the HEAD (or\n>> the index) to see if the prefix string matches an entity at the\n>> current directory level, and then (2) to prefix the result of the\n>> previous step with \"/$(git rev-parse --show-prefix)\" work? That is\n>> something like this:\n>>\n>> $ cd t\n>> $ git sparse-checkout add help\n>> ->\n>> $ git sparse-checkout add /t/helper/\n>\n> I thought bash-completion was only for completions, not for startings\n> as well. Was I mistaken?\n\nTo my mind, the completion is what I as an end user get when I type\n to help me formulate input that is acceptable by the command.\nAs I said, I consider it a bug (or UI mistake) in the a command if\nit pretends to work inside a subdirecctory but complains when it is\ngiven a path relative to the current directory, so I'd rather prefer\nthe approach to \"fix\" the underlying command, but if that is too\nmuch work or cannot be done for whatever reason, the second best\nwould be to turn whatever we can do to help the end-user input into\na form that is accepted by the command without changing what the\ninput means. If it takes more than \"appending at the end\", that is\nfine, at least by me as an end user.\n\nIf you are saying \"completion code can only append at the end\nbecause we can only return strings to be appended, not the entire\nstrings, to the readline machinery, so mucking with the start of the\nstring is not doable\", then sorry---I accept that what we cannot do\ncannot be done, and in that case you are \"not mistaken\".\n\nBut from the existing use of COMPREPLY[], it didn't look that way\n(it seems __gitcomp is equipped to take fixed prefix to all\ncandidates by passing it in $2 and used to complete names of\nconfiguration variables in a section, but it seems to me that it can\nbe repurposed when prefixing \"$(git rev-parse --show-prefix)\" to a\ngiven pathname relative to the $cwd). And if that can be done, then\nyou are \"not mistaken\", but merely being dogmatic and limiting what\nyour code can do yourself.\n\n>> Another more fundamental approach to avoid \"confusion\" this bullet\n>> item tries to side step might be to *fix* the command that gets\n>> completed. As \"git sparse-checkout --help\" is marked as\n>> EXPERIMENTAL in capital letters, we should be able to say \"what was\n>> traditionally known as 'add' is from now on called 'add-pattern' and\n>> command line completion would not get in the way; the 'add'\n>> subcommand now takes only literal paths, not patterns, that are\n>> relative to the current directory\" if we wanted to.\n>\n> That's interesting...but it opens up a new can of worms:\n> * Would we also need both `set-patterns` and `set`, in addition to\n> `add-patterns` and `add`?\n\nIf \"set\" has a similar UI issue that confuses end-users, then sure,\nI do not see a reason why we want to leave it confusing---the\nexperimental labelling is to allow us to fix these warts more\neasily, no?\n\n> * In cone mode, the paths passed are literal directories (and only\n> directories; no individual files), but the thing added is a\n> telescoping \"cone\" of leading directories as well. Does this make it\n> potentially confusing to users to say that `add` only takes literal\n> paths?\n\nI do not know.\n\n> * In cone mode (the default), should `add-patterns` just be an\n> error, since no pattern specification is allowed?\n\nI do not really care. \"add-patterns\" is a potential tool you can\nuse to reduce friction while fixing the UI warts in an experimental\ncommand.\n\n> * In the git-sparse-checkout manual, for performance reasons, we\n> recommend users _not_ specify individual paths in non-cone mode.\n> Would our recommendation then be to just not use `add` or `set` and\n> only use `add-patterns` and `set-patterns`?\n\nVery likely. If the desired behaviour from the command can only be\nhad by castrating features, then such a recommendation would not\nmean much to end-users anyway, though.\n\n> If so, what have we\n> accomplished by adding the new names?\n\nIt is valuable for those who do need to go against recommendation\n(because the recommendation robs usability from them way too much),\nwill have much less confusing and working completion when they use\n'add' or 'set', no?\n\n> Maybe I'm missing something about your suggestion, but this seems much\n> more complex than the simple solution we implemented in bb8b5e9a90d\n> (\"sparse-checkout: pay attention to prefix for {set, add}\",\n> 2022-02-19) for the !core_sparse_checkout_cone case.\n\nOh, if we do honor the $(git rev-parse --show-prefix), then that\nchanges the equation somewhat. I got an impression from your log\nmessage or cover letter that it wasn't the case, and that was where\nthe \"if the command is so broken, then completion can add it for the\nuser\" and \"if the command is so broken, then fix it to take relative\npaths\" came from.\n\n"},{"id":"485312","messageId":"97e20e3b99d8261815450e8a7c137938caaf5a6e.1701583024.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v3.git.1701583024.gitgitgadget@gmail.com","subject":"[PATCH v3 1/4] completion: squelch stray errors in sparse-checkout completion","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-12-03T05:57:01Z","receivedAt":"2023-12-03T05:57:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIf, in the root of a project, one types\n\n git sparse-checkout set --cone ../\n\nthen an error message of the form\n\n fatal: ../: '../' is outside repository at '/home/newren/floss/git'\n\nis written to stderr, which munges the users view of their own command.\nSquelch such messages by using the __git() wrapper, designed for this\npurpose; see commit e15098a314 (completion: consolidate silencing errors\nfrom git commands, 2017-02-03) for more on the wrapper.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 13a39ebd2e7..b8661701718 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3084,7 +3084,7 @@ __gitcomp_directories ()\n \t\t\tCOMPREPLY+=(\"$c/\")\n \t\t\t_found=1\n \t\tfi\n-\tdone < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)\n+\tdone < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir)\n \n \tif [[ $_found == 0 ]] && [[ \"$cur\" =~ /$ ]]; then\n \t\t# No possible further completions any deeper, so assume we're at\n-- \ngitgitgadget\n\n"},{"id":"485313","messageId":"pull.1349.v3.git.1701583024.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v2.git.1700985086.gitgitgadget@gmail.com","subject":"[PATCH v3 0/4] Sparse checkout completion fixes","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-12-03T05:57:00Z","receivedAt":"2023-12-03T05:57:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This fixes a few issues with tab completion for the sparse-checkout command,\nspecifically with the \"add\" and \"set\" subcommands.\n\nChanges since v2:\n\n * For patch 4, provide completion (and startings) of arguments into\n patterns that match files in the index -- as suggested by Junio.\n\nChanges since v1:\n\n * Use __git wrapper function to squelch errors, as suggested by SZEDER\n Gábor\n * note that we could use the index or HEAD for the more involved solution\n in patch 4.\n\n[1] https://lore.kernel.org/git/xmqqv8yjz5us.fsf@gitster.g/\n\nElijah Newren (4):\n completion: squelch stray errors in sparse-checkout completion\n completion: fix logic for determining whether cone mode is active\n completion: avoid misleading completions in cone mode\n completion: avoid user confusion in non-cone mode\n\n contrib/completion/git-completion.bash | 123 ++++++++++++++++++++++++-\n t/t9902-completion.sh | 9 +-\n 2 files changed, 127 insertions(+), 5 deletions(-)\n\n\nbase-commit: 564d0252ca632e0264ed670534a51d18a689ef5d\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1349%2Fnewren%2Fsparse-checkout-completion-fixes-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1349/newren/sparse-checkout-completion-fixes-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1349\n\nRange-diff vs v2:\n\n 1: 97e20e3b99d = 1: 97e20e3b99d completion: squelch stray errors in sparse-checkout completion\n 2: 212ba35ed46 = 2: 212ba35ed46 completion: fix logic for determining whether cone mode is active\n 3: 1cbbcd9097c = 3: 1cbbcd9097c completion: avoid misleading completions in cone mode\n 4: 604f21dc827 < -: ----------- completion: avoid user confusion in non-cone mode\n -: ----------- > 4: 89501b366ff completion: avoid user confusion in non-cone mode\n\n-- \ngitgitgadget\n"},{"id":"485314","messageId":"212ba35ed469b63fc75c51b61588025c303a162d.1701583024.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v3.git.1701583024.gitgitgadget@gmail.com","subject":"[PATCH v3 2/4] completion: fix logic for determining whether cone mode is active","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-12-03T05:57:02Z","receivedAt":"2023-12-03T05:57:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\n_git_sparse_checkout() was checking whether we were in cone mode by\nchecking whether either:\n\n A) core.sparseCheckoutCone was \"true\"\n B) \"--cone\" was specified on the command line\n\nThis code has 2 bugs I didn't catch in my review at the time\n\n 1) core.sparseCheckout must be \"true\" for core.sparseCheckoutCone to\n be relevant (which matters since \"git sparse-checkout disable\"\n only unsets core.sparseCheckout, not core.sparseCheckoutCone)\n 2) The presence of \"--no-cone\" should override any config setting\n\nFurther, I forgot to update this logic as part of 2d95707a02\n(\"sparse-checkout: make --cone the default\", 2022-04-22) for the new\ndefault.\n\nUpdate the code for the new default and make it be more careful in\ndetermining whether to complete based on cone mode or non-cone mode.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b8661701718..7aa66c19ede 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3097,6 +3097,7 @@ _git_sparse_checkout ()\n {\n \tlocal subcommands=\"list init set disable add reapply\"\n \tlocal subcommand=\"$(__git_find_on_cmdline \"$subcommands\")\"\n+\tlocal using_cone=true\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\n \t\treturn\n@@ -3107,8 +3108,15 @@ _git_sparse_checkout ()\n \t\t__gitcomp_builtin sparse-checkout_$subcommand \"\" \"--\"\n \t\t;;\n \tset,*|add,*)\n-\t\tif [ \"$(__git config core.sparseCheckoutCone)\" == \"true\" ] ||\n-\t\t[ -n \"$(__git_find_on_cmdline --cone)\" ]; then\n+\t\tif [[ \"$(__git config core.sparseCheckout)\" == \"true\" &&\n+\t\t \"$(__git config core.sparseCheckoutCone)\" == \"false\" &&\n+\t\t -z \"$(__git_find_on_cmdline --cone)\" ]]; then\n+\t\t\tusing_cone=false\n+\t\tfi\n+\t\tif [[ -n \"$(__git_find_on_cmdline --no-cone)\" ]]; then\n+\t\t\tusing_cone=false\n+\t\tfi\n+\t\tif [[ \"$using_cone\" == \"true\" ]]; then\n \t\t\t__gitcomp_directories\n \t\tfi\n \tesac\n-- \ngitgitgadget\n\n"},{"id":"485315","messageId":"1cbbcd9097c96b7646fb0c756c3964413f2a021f.1701583024.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v3.git.1701583024.gitgitgadget@gmail.com","subject":"[PATCH v3 3/4] completion: avoid misleading completions in cone mode","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-12-03T05:57:03Z","receivedAt":"2023-12-03T05:57:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe \"set\" and \"add\" subcommands of \"sparse-checkout\", when in cone mode,\nshould only complete on directories. For bash_completion in general,\nwhen no completions are returned for any subcommands, it will often fall\nback to standard completion of files and directories as a substitute.\nThat is not helpful here. Since we have already looked for all valid\ncompletions, if none are found then falling back to standard bash file\nand directory completion is at best actively misleading. In fact, there\nare three different ways it can be actively misleading. Add a long\ncomment in the code about how that fallback behavior can deceive, and\ndisable the fallback by returning a fake result as the sole completion.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7aa66c19ede..c614e5d4f07 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3090,6 +3090,26 @@ __gitcomp_directories ()\n \t\t# No possible further completions any deeper, so assume we're at\n \t\t# a leaf directory and just consider it complete\n \t\t__gitcomp_direct_append \"$cur \"\n+\telif [[ $_found == 0 ]]; then\n+\t\t# No possible completions found. Avoid falling back to\n+\t\t# bash's default file and directory completion, because all\n+\t\t# valid completions have already been searched and the\n+\t\t# fallbacks can do nothing but mislead. In fact, they can\n+\t\t# mislead in three different ways:\n+\t\t# 1) Fallback file completion makes no sense when asking\n+\t\t# for directory completions, as this function does.\n+\t\t# 2) Fallback directory completion is bad because\n+\t\t# e.g. \"/pro\" is invalid and should NOT complete to\n+\t\t# \"/proc\".\n+\t\t# 3) Fallback file/directory completion only completes\n+\t\t# on paths that exist in the current working tree,\n+\t\t# i.e. which are *already* part of their\n+\t\t# sparse-checkout. Thus, normal file and directory\n+\t\t# completion is always useless for \"git\n+\t\t# sparse-checkout add\" and is also probelmatic for\n+\t\t# \"git sparse-checkout set\" unless using it to\n+\t\t# strictly narrow the checkout.\n+\t\tCOMPREPLY=( \"\" )\n \tfi\n }\n \n-- \ngitgitgadget\n\n"},{"id":"485316","messageId":"89501b366ff0476c1b3d36ff9b6b7c80fa6fc98f.1701583024.git.gitgitgadget@gmail.com","threadId":"60550","inReplyTo":"pull.1349.v3.git.1701583024.gitgitgadget@gmail.com","subject":"[PATCH v3 4/4] completion: avoid user confusion in non-cone mode","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-12-03T05:57:04Z","receivedAt":"2023-12-03T05:57:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIt is tempting to think of \"files and directories\" of the current\ndirectory as valid inputs to the add and set subcommands of git\nsparse-checkout. However, in non-cone mode, they often aren't and using\nthem as potential completions leads to *many* forms of confusion:\n\nIssue #1. It provides the *wrong* files and directories.\n\nFor\n git sparse-checkout add\nwe always want to add files and directories not currently in our sparse\ncheckout, which means we want file and directories not currently present\nin the current working tree. Providing the files and directories\ncurrently present is thus always wrong.\n\nFor\n git sparse-checkout set\nwe have a similar problem except in the subset of cases where we are\ntrying to narrow our checkout to a strict subset of what we already\nhave. That is not a very common scenario, especially since it often\ndoes not even happen to be true for the first use of the command; for\nyears we required users to create a sparse-checkout via\n git sparse-checkout init\n git sparse-checkout set \n(or use a clone option that did the init step for you at clone time).\nThe init command creates a minimal sparse-checkout with just the\ntop-level directory present, meaning the set command has to be used to\nexpand the checkout. Thus, only in a special and perhaps unusual cases\nwould any of the suggestions from normal file and directory completion\nbe appropriate.\n\nIssue #2: Suggesting patterns that lead to warnings is unfriendly.\n\nIf the user specifies any regular file and omits the leading '/', then\nthe sparse-checkout command will warn the user that their command is\nproblematic and suggest they use a leading slash instead.\n\nIssue #3: Completion gets confused by leading '/', and provides wrong paths.\n\nUsers often want to anchor their patterns to the toplevel of the\nrepository, especially when listing individual files. There are a\nnumber of reasons for this, but notably even sparse-checkout encourages\nthem to do so (as noted above). However, if users do so (via adding a\nleading '/' to their pattern), then bash completion will interpret the\nleading slash not as a request for a path at the toplevel of the\nrepository, but as a request for a path at the root of the filesytem.\nThat means at best that completion cannot help with such paths, and if\nit does find any completions, they are almost guaranteed to be wrong.\n\nIssue #4: Suggesting invalid patterns from subdirectories is unfriendly.\n\nThere is no per-directory equivalent to .gitignore with\nsparse-checkouts. There is only a single worktree-global\n$GIT_DIR/info/sparse-checkout file. As such, paths to files must be\nspecified relative to the toplevel of a repository. Providing\nsuggestions of paths that are relative to the current working directory,\nas bash completion defaults to, is wrong when the current working\ndirectory is not the worktree toplevel directory.\n\nIssue #5: Paths with special characters will be interpreted incorrectly\n\nThe entries in the sparse-checkout file are patterns, not paths. While\nmost paths also qualify as patterns (though even in such cases it would\nbe better for users to not use them directly but prefix them with a\nleading '/'), there are a variety of special characters that would need\nspecial escaping beyond the normal shell escaping: '*', '?', '\\', '[',\n']', and any leading '#' or '!'. If completion suggests any such paths,\nusers will likely expect them to be treated as an exact path rather than\nas a pattern that might match some number of files other than 1.\n\nHowever, despite the first four issues, we can note that _if_ users are\nusing tab completion, then they are probably trying to specify a path in\nthe index. As such, we transform their argument into a top-level-rooted\npattern that matches such a file. For example, if they type:\n git sparse-checkout add Make\nwe could \"complete\" to\n git sparse-checkout add /Makefile\nor, if they ran from the Documentation/technical/ subdirectory:\n git sparse-checkout add m\nwe could \"complete\" it to:\n git sparse-checkout add /Documentation/technical/multi-pack-index.txt\nNote in both cases I use \"complete\" in quotes, because we actually add\ncharacters both before and after the argument in question, so we are\nkind of abusing \"bash completions\" to be \"bash completions AND\nbeginnings\".\n\nThe fifth issue is a bit stickier, especially when you consider that we\nnot only need to deal with escaping issues because of special meanings\nof patterns in sparse-checkout & gitignore files, but also that we need\nto consider escaping issues due to ls-files needing to sometimes quote\nor escape characters, and because the shell needs to escape some\ncharacters. The multiple interacting forms of escaping could get ugly;\nthis patch makes no attempt to do so and simply documents that we\ndecided to not deal with those corner cases for now but at least get the\ncommon cases right.\n\nSigned-off-by: Elijah Newren \n---\n contrib/completion/git-completion.bash | 89 ++++++++++++++++++++++++++\n t/t9902-completion.sh | 9 ++-\n 2 files changed, 96 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c614e5d4f07..185b47d802f 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -3113,6 +3113,93 @@ __gitcomp_directories ()\n \tfi\n }\n \n+# In non-cone mode, the arguments to {set,add} are supposed to be\n+# patterns, relative to the toplevel directory. These can be any kind\n+# of general pattern, like 'subdir/*.c' and we can't complete on all\n+# of those. However, if the user presses Tab to get tab completion, we\n+# presume that they are trying to provide a pattern that names a specific\n+# path.\n+__gitcomp_slash_leading_paths ()\n+{\n+\tlocal dequoted_word pfx=\"\" cur_ toplevel\n+\n+\t# Since we are dealing with a sparse-checkout, subdirectories may not\n+\t# exist in the local working copy. Therefore, we want to run all\n+\t# ls-files commands relative to the repository toplevel.\n+\ttoplevel=\"$(git rev-parse --show-toplevel)/\"\n+\n+\t__git_dequote \"$cur\"\n+\n+\t# If the paths provided by the user already start with '/', then\n+\t# they are considered relative to the toplevel of the repository\n+\t# already. If they do not start with /, then we need to adjust\n+\t# them to start with the appropriate prefix.\n+\tcase \"$cur\" in\n+\t/*)\n+\t\tcur=\"${cur:1}\"\n+\t\t;;\n+\t*)\n+\t\tpfx=\"$(__git rev-parse --show-prefix)\"\n+\tesac\n+\n+\t# Since sparse-index is limited to cone-mode, in non-cone-mode the\n+\t# list of valid paths is precisely the cached files in the index.\n+\t#\n+\t# NEEDSWORK:\n+\t# 1) We probably need to take care of cases where ls-files\n+\t# responds with special quoting.\n+\t# 2) We probably need to take care of cases where ${cur} has\n+\t# some kind of special quoting.\n+\t# 3) On top of any quoting from 1 & 2, we have to provide an extra\n+\t# level of quoting for any paths that contain a '*', '?', '\\',\n+\t# '[', ']', or leading '#' or '!' since those will be\n+\t# interpreted by sparse-checkout as something other than a\n+\t# literal path character.\n+\t# Since there are two types of quoting here, this might get really\n+\t# complex. For now, just punt on all of this...\n+\tcompletions=\"$(__git -C \"${toplevel}\" -c core.quotePath=false \\\n+\t\t\t ls-files --cached -- \"${pfx}${cur}*\" \\\n+\t\t\t | sed -e s%^%/% -e 's%$% %')\"\n+\t# Note, above, though that we needed all of the completions to be\n+\t# prefixed with a '/', and we want to add a space so that bash\n+\t# completion will actually complete an entry and let us move on to\n+\t# the next one.\n+\n+\t# Return what we've found.\n+\tif test -n \"$completions\"; then\n+\t\t# We found some completions; return them\n+\t\tlocal IFS=$'\\n'\n+\t\tCOMPREPLY=($completions)\n+\telse\n+\t\t# Do NOT fall back to bash-style all-local-files-and-dirs\n+\t\t# when we find no match. Such options are worse than\n+\t\t# useless:\n+\t\t# 1. \"git sparse-checkout add\" needs paths that are NOT\n+\t\t# currently in the working copy. \"git\n+\t\t# sparse-checkout set\" does as well, except in the\n+\t\t# special cases when users are only trying to narrow\n+\t\t# their sparse checkout to a subset of what they\n+\t\t# already have.\n+\t\t#\n+\t\t# 2. A path like '.config' is ambiguous as to whether\n+\t\t# the user wants all '.config' files throughout the\n+\t\t# tree, or just the one under the current directory.\n+\t\t# It would result in a warning from the\n+\t\t# sparse-checkout command due to this. As such, all\n+\t\t# completions of paths should be prefixed with a\n+\t\t# '/'.\n+\t\t#\n+\t\t# 3. We don't want paths prefixed with a '/' to\n+\t\t# complete files in the system root directory, we\n+\t\t# want it to complete on files relative to the\n+\t\t# repository root.\n+\t\t#\n+\t\t# As such, make sure that NO completions are offered rather\n+\t\t# than falling back to bash's default completions.\n+\t\tCOMPREPLY=( \"\" )\n+\tfi\n+}\n+\n _git_sparse_checkout ()\n {\n \tlocal subcommands=\"list init set disable add reapply\"\n@@ -3138,6 +3225,8 @@ _git_sparse_checkout ()\n \t\tfi\n \t\tif [[ \"$using_cone\" == \"true\" ]]; then\n \t\t\t__gitcomp_directories\n+\t\telse\n+\t\t\t __gitcomp_slash_leading_paths\n \t\tfi\n \tesac\n }\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex a7c3b4eb63a..bbd17f296e4 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1571,7 +1571,7 @@ test_expect_success FUNNYNAMES,!CYGWIN 'cone mode sparse-checkout completes dire\n \t)\n '\n \n-test_expect_success 'non-cone mode sparse-checkout uses bash completion' '\n+test_expect_success 'non-cone mode sparse-checkout gives rooted paths' '\n \t# reset sparse-checkout repo to non-cone mode\n \tgit -C sparse-checkout sparse-checkout disable &&\n \tgit -C sparse-checkout sparse-checkout set --no-cone &&\n@@ -1581,7 +1581,12 @@ test_expect_success 'non-cone mode sparse-checkout uses bash completion' '\n \t\t# expected to be empty since we have not configured\n \t\t# custom completion for non-cone mode\n \t\ttest_completion \"git sparse-checkout set f\" <<-\\EOF\n-\n+\t\t/folder1/0/1/t.txt \n+\t\t/folder1/expected \n+\t\t/folder1/out \n+\t\t/folder1/out_sorted \n+\t\t/folder2/0/t.txt \n+\t\t/folder3/t.txt \n \t\tEOF\n \t)\n '\n-- \ngitgitgadget\n"},{"id":"485317","messageId":"CABPp-BHDx+e6dS9zNrS87-ROvr08p4YNqs5KUHF3E67DJRWihg@mail.gmail.com","threadId":"60550","inReplyTo":"xmqq34ws3rfa.fsf@gitster.g","subject":"Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-12-03T05:57:46Z","receivedAt":"2023-12-03T05:58:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, Nov 26, 2023 at 5:39 PM Junio C Hamano wrote:\n[...]\n> >> Hmph, would an obvious alternative to (1) check against the HEAD (or\n> >> the index) to see if the prefix string matches an entity at the\n> >> current directory level, and then (2) to prefix the result of the\n> >> previous step with \"/$(git rev-parse --show-prefix)\" work? That is\n> >> something like this:\n> >>\n> >> $ cd t\n> >> $ git sparse-checkout add help\n> >> ->\n> >> $ git sparse-checkout add /t/helper/\n> >\n> > I thought bash-completion was only for completions, not for startings\n> > as well. Was I mistaken?\n>\n> To my mind, the completion is what I as an end user get when I type\n> to help me formulate input that is acceptable by the command.\n> As I said, I consider it a bug (or UI mistake) in the a command if\n> it pretends to work inside a subdirecctory but complains when it is\n> given a path relative to the current directory, so I'd rather prefer\n> the approach to \"fix\" the underlying command, but if that is too\n> much work or cannot be done for whatever reason, the second best\n> would be to turn whatever we can do to help the end-user input into\n> a form that is accepted by the command without changing what the\n> input means. If it takes more than \"appending at the end\", that is\n> fine, at least by me as an end user.\n>\n> If you are saying \"completion code can only append at the end\n> because we can only return strings to be appended, not the entire\n> strings, to the readline machinery, so mucking with the start of the\n> string is not doable\", then sorry---I accept that what we cannot do\n> cannot be done, and in that case you are \"not mistaken\".\n\nThis was what I thought; that bash completion didn't support this.\n\n> But from the existing use of COMPREPLY[], it didn't look that way\n> (it seems __gitcomp is equipped to take fixed prefix to all\n> candidates by passing it in $2 and used to complete names of\n> configuration variables in a section, but it seems to me that it can\n> be repurposed when prefixing \"$(git rev-parse --show-prefix)\" to a\n> given pathname relative to the $cwd).\n\nOoh, that's really interesting; I had no idea it had this kind of\nflexibility. It does feel like we're abusing \"bash completions\" to be\nboth \"bash completions AND startings\", but I agree that this is a case\nwhere it makes sense to do so.\n\nI changed patch 4 to implement this for non-cone mode, and submitted\nv3 with that change.\n"},{"id":"485333","messageId":"xmqqv89f4eck.fsf@gitster.g","threadId":"60550","inReplyTo":"89501b366ff0476c1b3d36ff9b6b7c80fa6fc98f.1701583024.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 4/4] completion: avoid user confusion in non-cone mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-03T13:15:07Z","receivedAt":"2023-12-03T13:15:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> @@ -1581,7 +1581,12 @@ test_expect_success 'non-cone mode sparse-checkout uses bash completion' '\n> \t\t# expected to be empty since we have not configured\n> \t\t# custom completion for non-cone mode\n> \t\ttest_completion \"git sparse-checkout set f\" <<-\\EOF\n> -\n> +\t\t/folder1/0/1/t.txt \n> +\t\t/folder1/expected \n> +\t\t/folder1/out \n> +\t\t/folder1/out_sorted \n> +\t\t/folder2/0/t.txt \n> +\t\t/folder3/t.txt \n> \t\tEOF\n\nThe \"test_completion\" helper strips \"Z\" at the end of its input\nlines so that a hunk like this can be written without having to\nworry about mail transport eating trailing whitespaces.\n\nI'll squash the following while queuing.\n\nThanks.\n\n t/t9902-completion.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 5634b78fc5..aa9a614de3 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1581,12 +1581,12 @@ test_expect_success 'non-cone mode sparse-checkout gives rooted paths' '\n \t\t# expected to be empty since we have not configured\n \t\t# custom completion for non-cone mode\n \t\ttest_completion \"git sparse-checkout set f\" <<-\\EOF\n-\t\t/folder1/0/1/t.txt\n-\t\t/folder1/expected\n-\t\t/folder1/out\n-\t\t/folder1/out_sorted\n-\t\t/folder2/0/t.txt\n-\t\t/folder3/t.txt\n+\t\t/folder1/0/1/t.txt Z\n+\t\t/folder1/expected Z\n+\t\t/folder1/out Z\n+\t\t/folder1/out_sorted Z\n+\t\t/folder2/0/t.txt Z\n+\t\t/folder3/t.txt Z\n \t\tEOF\n \t)\n '\n"}]}