From patchwork Sun Dec 3 05:57:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13477194 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="AKnpva02" Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1B1AE106 for ; Sat, 2 Dec 2023 21:57:08 -0800 (PST) Received: by mail-wr1-x435.google.com with SMTP id ffacd0b85a97d-3332efd75c9so2049705f8f.2 for ; Sat, 02 Dec 2023 21:57:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701583026; x=1702187826; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=XlKqH4Ex5Hz0DcVmbkbwp1b+sjR9qljU7Rkjy86OiGo=; b=AKnpva02ZHvr94gAA11r8By1cLVndx9Cn8lvZgIGj7YaDkbEwcKEMnZf4IIXlIeE0S 8AJCp932yjbAmGUn/qm4YXXJEcV3mJqMIPeC/GLPxufUaQmcQtgJ+H4Y/QDC2+3a6dJu VHjxdc/QbrOOvjmEFd5nS8jVJEnMzSvhi8c5AIQnYSwTL9KSGLpSIYf2e1PN3SO1TGbl 4PNFNZL61AztGKPb4w7HDep0qWmE4iAbyYH8SipPnhYzTw/Fni5JDKufOXieW3tXcwi9 FTzmUtwwJ/eQGfIU2tTP1ROJITqUP64Nl4kd6lyEy12rdoabtAgq6AIuZx+wqMbDzJ/J 9s0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701583026; x=1702187826; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XlKqH4Ex5Hz0DcVmbkbwp1b+sjR9qljU7Rkjy86OiGo=; b=MyiJD0y/4tVKOVUANgmDQ56WobeVw3T1o7zvffu7Ps0pneSYl6ao+r7fm4Z1C9A6Fv wfA+LWUN2veGXetyHUCGgRj6JF9vwzGdD+QdUVnGhL0/O8kItzTkzR0kRke9TOJ7JDCq jzklhWD/3CV4fS+uGyVQsjUkge4PcWHqEY2zu1YOQ1QOVMh5U9Uq1Kc1RJh+MagEW/xQ ba388yx2ZpP7BTKuuFOU6oHnAXhULrhZOUzXW1CCORIRUpGZhc5PhP+5IssU2qHbgpF5 VO1przLdf0bY7fNzWpFjmROaMA5mOxmlmyFJyDoCLlSg4Q3tuewkFk6wZSLyC/uTLVBM pKyw== X-Gm-Message-State: AOJu0Yx90axRgpr7PtN2Nmidksh/s1KzsQNuZzX+/QBGPQJVFHjOgnCr aV54o0hxzuVwQZIq/5Dbd867mhCLvbg= X-Google-Smtp-Source: AGHT+IFlSHziKOctle1n477Fqs+7Se7vIWWt7gPVh/hWqEN14CMjau06/4NJgMQebn1+t0XsTgMbpQ== X-Received: by 2002:adf:b608:0:b0:333:3802:9e07 with SMTP id f8-20020adfb608000000b0033338029e07mr553087wre.136.1701583026030; Sat, 02 Dec 2023 21:57:06 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f11-20020adfb60b000000b00333415f1c80sm1123611wre.70.2023.12.02.21.57.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 Dec 2023 21:57:05 -0800 (PST) Message-ID: <97e20e3b99d8261815450e8a7c137938caaf5a6e.1701583024.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sun, 03 Dec 2023 05:57:01 +0000 Subject: [PATCH v3 1/4] completion: squelch stray errors in sparse-checkout completion Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , SZEDER =?utf-8?b?R8OhYm9y?= , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren If, in the root of a project, one types git sparse-checkout set --cone ../ then an error message of the form fatal: ../: '../' is outside repository at '/home/newren/floss/git' is written to stderr, which munges the users view of their own command. Squelch such messages by using the __git() wrapper, designed for this purpose; see commit e15098a314 (completion: consolidate silencing errors from git commands, 2017-02-03) for more on the wrapper. Signed-off-by: Elijah Newren --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 13a39ebd2e7..b8661701718 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -3084,7 +3084,7 @@ __gitcomp_directories () COMPREPLY+=("$c/") _found=1 fi - done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir) + done < <(__git ls-tree -z -d --name-only HEAD $_tmp_dir) if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then # No possible further completions any deeper, so assume we're at From patchwork Sun Dec 3 05:57:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13477196 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="YdXz95vt" Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BA654114 for ; Sat, 2 Dec 2023 21:57:08 -0800 (PST) Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-40bda47c489so12377675e9.3 for ; Sat, 02 Dec 2023 21:57:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701583027; x=1702187827; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=/gDsstomIT0hYG7RuKOvZTnJ9cX/gRG40cwI2P+Nco0=; b=YdXz95vtKrkrwjNGqkrYul0v0M40E4CvKsByPEOIMhI78KmB/dIllmWXDXhN6MJ9eD ysUbvEwo/X15YtVENAl3uSiuUjbfhgZcBgnWlSVe6d9HeU8uU/RA0sYF0JqA1e87no9A bq2GKylJ3HyVr7xW4h/5tQWSCqCqb4P8CPZweM8zN4tAqGLmC5mwbXLDHf1B1ZDI4jVZ QAxzr4E78mOZ2e3leMfJ2HPls8KoIVYWXdtQ+ONGAPJj1I1dXSd3L0IHMrMFToXMprsI dUvydzy0PvopzBFegVO7EUQ9DcDBH8mI5wd7MnE+eU+v89WUF0j9hkdDZrnpn8dx/b2I JE0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701583027; x=1702187827; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/gDsstomIT0hYG7RuKOvZTnJ9cX/gRG40cwI2P+Nco0=; b=HtRJFvU32iaf6KAjPN3CpgHKVPIbeTy25skZ7glENqn635emLfweVa298zlVRVuX5T uKwkC/63WpZw9cwdf7VUlRT/rcYCVDDWOCIW+EHzikI9HKpw2ZDZnSxKqXSODR4pmWMU n2DOItmq56wra/h4lsFw5F3LMqSDCdD3i9MaFz7gCd2OHLSSmJR7QMaHj7Xdt57g7k8V YCwYK8O5hTUv5Y7R9WQ1A5NvwfSRAZnxG42aT86+oPLLyhny1ijtzgLZ3hAWdRbOlR/H X8hekxISv9Fq4YBGbF3U0IuoiGd+MGfj6ILSYS8Bq6ZNShww64quu2CJ2CMOolx6xhy+ ni0A== X-Gm-Message-State: AOJu0YzigxPNWWCUJVXVXcMIctTF9S1ukeevGvocXOkxHJW9kvxieDmZ Gmuj1rWQKvRozq5pW05BkMEfcm3NfvY= X-Google-Smtp-Source: AGHT+IGiIdZQLNewzHw6vr+/5wnMIDcjhwn1N/A2lWvCPIKs1ZBv4rfvgZfRk89J93mD4r8reLQQng== X-Received: by 2002:a05:600c:1383:b0:40b:5e59:c57f with SMTP id u3-20020a05600c138300b0040b5e59c57fmr1597868wmf.169.1701583026998; Sat, 02 Dec 2023 21:57:06 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id p7-20020a05600c358700b0040849ce7116sm14404177wmq.43.2023.12.02.21.57.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 Dec 2023 21:57:06 -0800 (PST) Message-ID: <212ba35ed469b63fc75c51b61588025c303a162d.1701583024.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sun, 03 Dec 2023 05:57:02 +0000 Subject: [PATCH v3 2/4] completion: fix logic for determining whether cone mode is active Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , SZEDER =?utf-8?b?R8OhYm9y?= , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren _git_sparse_checkout() was checking whether we were in cone mode by checking whether either: A) core.sparseCheckoutCone was "true" B) "--cone" was specified on the command line This code has 2 bugs I didn't catch in my review at the time 1) core.sparseCheckout must be "true" for core.sparseCheckoutCone to be relevant (which matters since "git sparse-checkout disable" only unsets core.sparseCheckout, not core.sparseCheckoutCone) 2) The presence of "--no-cone" should override any config setting Further, I forgot to update this logic as part of 2d95707a02 ("sparse-checkout: make --cone the default", 2022-04-22) for the new default. Update the code for the new default and make it be more careful in determining whether to complete based on cone mode or non-cone mode. Signed-off-by: Elijah Newren --- contrib/completion/git-completion.bash | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index b8661701718..7aa66c19ede 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -3097,6 +3097,7 @@ _git_sparse_checkout () { local subcommands="list init set disable add reapply" local subcommand="$(__git_find_on_cmdline "$subcommands")" + local using_cone=true if [ -z "$subcommand" ]; then __gitcomp "$subcommands" return @@ -3107,8 +3108,15 @@ _git_sparse_checkout () __gitcomp_builtin sparse-checkout_$subcommand "" "--" ;; set,*|add,*) - if [ "$(__git config core.sparseCheckoutCone)" == "true" ] || - [ -n "$(__git_find_on_cmdline --cone)" ]; then + if [[ "$(__git config core.sparseCheckout)" == "true" && + "$(__git config core.sparseCheckoutCone)" == "false" && + -z "$(__git_find_on_cmdline --cone)" ]]; then + using_cone=false + fi + if [[ -n "$(__git_find_on_cmdline --no-cone)" ]]; then + using_cone=false + fi + if [[ "$using_cone" == "true" ]]; then __gitcomp_directories fi esac From patchwork Sun Dec 3 05:57:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13477197 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="llfk56Ls" Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 23100119 for ; Sat, 2 Dec 2023 21:57:10 -0800 (PST) Received: by mail-wr1-x433.google.com with SMTP id ffacd0b85a97d-3333127685bso1809120f8f.0 for ; Sat, 02 Dec 2023 21:57:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701583028; x=1702187828; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=yyzT933Lv+I2eJ6KcLdgufUGswSUXXJts+i+4YrqxEk=; b=llfk56LszCUH5/sP6RGMooo+CCyRelVnk5VvqH7e6Uetq2lVCz7pp4r8OnnifCkcE9 udiads3HS4pc4g2cgOWxtsZaQGVPKGBodS9ui8SntOPjuERQNQyxaPEcVDV4JQ7cSKr8 c7Sv5vz7zlFOaQ5jH0oq1v/W1e+zvjcCYwzX1pA1BHlJT+FFDXPVtkNLKCQMcz0Db50N MSG8hR09kuhAbL2tEKBg8kLx27fVb5I8gJYqfyMnHLEkbIAu1oKnfBsEdkhJ4af0mRjx p62mRPU3S9Ws03OVRIouwBIkKHX3QmdmyoRVInTW85eo89N2IsTGxViJj+4NNZfmVvdY 7DfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701583028; x=1702187828; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yyzT933Lv+I2eJ6KcLdgufUGswSUXXJts+i+4YrqxEk=; b=iDfmitVU5G2dGnghDXWiwa0Th3pPh8uab1QrBg1sHblJDPfTF3XbmRbKdCHmFZFUVd DtfyJ04l/U1k/i8PlL+CkW8mILN4ZDVF62HBgi8pWOBKTgPyMAFvi6cRVWiLMboiJCTK dPbfFuIKL3AKXDjkcqV4LrQNVWzr7+rcsXXEbn6ZThcnPYy10N8AOGv3v3QwxC+5pDUo 2Cb8Mx3VGj3Gswx1614bOavb/HmnPd/pr4QHAMHUtYhlqfn1dAD/WaqDaDF6TR87P84q l8MPnLjn701fgcZVPsx1MWPpatwmbpnN27SZaIgxKta7de+cyDC/AsqQ0aMkO6O6cnm8 AQgQ== X-Gm-Message-State: AOJu0Yz6v21LvnMFnf14WAIBMJeSovOdVvz8Gp3ra2+T0ToHS8hnIxcQ dOgPvBWgNdiWCAgRjADHuetw1GIPbJo= X-Google-Smtp-Source: AGHT+IE3ioZLP1+M9OxYsCi/ceH+R98Dr7NrbAtOpK/VKwhMsYKKziLYLNuSnxqYKwdV/rJHTvzpIg== X-Received: by 2002:adf:f402:0:b0:333:2fd2:3bcb with SMTP id g2-20020adff402000000b003332fd23bcbmr1626754wro.132.1701583027668; Sat, 02 Dec 2023 21:57:07 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id z16-20020a5d6550000000b0033340c12012sm1414633wrv.3.2023.12.02.21.57.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 Dec 2023 21:57:07 -0800 (PST) Message-ID: <1cbbcd9097c96b7646fb0c756c3964413f2a021f.1701583024.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sun, 03 Dec 2023 05:57:03 +0000 Subject: [PATCH v3 3/4] completion: avoid misleading completions in cone mode Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , SZEDER =?utf-8?b?R8OhYm9y?= , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren The "set" and "add" subcommands of "sparse-checkout", when in cone mode, should only complete on directories. For bash_completion in general, when no completions are returned for any subcommands, it will often fall back to standard completion of files and directories as a substitute. That is not helpful here. Since we have already looked for all valid completions, if none are found then falling back to standard bash file and directory completion is at best actively misleading. In fact, there are three different ways it can be actively misleading. Add a long comment in the code about how that fallback behavior can deceive, and disable the fallback by returning a fake result as the sole completion. Signed-off-by: Elijah Newren --- contrib/completion/git-completion.bash | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 7aa66c19ede..c614e5d4f07 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -3090,6 +3090,26 @@ __gitcomp_directories () # No possible further completions any deeper, so assume we're at # a leaf directory and just consider it complete __gitcomp_direct_append "$cur " + elif [[ $_found == 0 ]]; then + # No possible completions found. Avoid falling back to + # bash's default file and directory completion, because all + # valid completions have already been searched and the + # fallbacks can do nothing but mislead. In fact, they can + # mislead in three different ways: + # 1) Fallback file completion makes no sense when asking + # for directory completions, as this function does. + # 2) Fallback directory completion is bad because + # e.g. "/pro" is invalid and should NOT complete to + # "/proc". + # 3) Fallback file/directory completion only completes + # on paths that exist in the current working tree, + # i.e. which are *already* part of their + # sparse-checkout. Thus, normal file and directory + # completion is always useless for "git + # sparse-checkout add" and is also probelmatic for + # "git sparse-checkout set" unless using it to + # strictly narrow the checkout. + COMPREPLY=( "" ) fi } From patchwork Sun Dec 3 05:57:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 13477198 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="nqoBoFKb" Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8B6F911A for ; Sat, 2 Dec 2023 21:57:10 -0800 (PST) Received: by mail-wm1-x32e.google.com with SMTP id 5b1f17b1804b1-40b4d9e81deso35599065e9.0 for ; Sat, 02 Dec 2023 21:57:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701583028; x=1702187828; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=PsoD7IRCHR+HJJL5qEUmaFKp8KbiCRqofZBD3iOLaxE=; b=nqoBoFKbQVAE8E39rhNXogLED1DgqknSf+WDGYMa0LisHhDy2DUB4S+jLXkTgxFAU6 AvZn6DrJskEZlCrX1CA5dTpEau+PrBRWhfpKa1b5MwKSHIY0CKtVCx1eh84iwArTBPEl oYI7JH6VqfIkvLC9lEr7WltIe6RhdQSDlqO2qt8CylQJ7hTiar+Yops/meXGkz8afSr/ c0bznnltk3SYi3EIH9duSNHbOu++iNUIpt5G35cqfoB6aoYYNQYegL5+8rs/mxT2zjnc /bt5BFrx0yRyf9bY951aLzA75VR01DdKSbK4Q/h0LQ/OoloKAvMOtitWD4aiHvoBSSUv 81MA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701583028; x=1702187828; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PsoD7IRCHR+HJJL5qEUmaFKp8KbiCRqofZBD3iOLaxE=; b=ftSS1IgS1V/hWz2EYa9yUw5OxZhMxYJO5OTVu4dVCZFqZwVwtXWK1KyGyNl+m19nPR FNGIdB2FiOW6Z/4yGDmZVQH6eKNi4MUMM4Enw3anOICWXHc2OJ8lJMgNqls9Zsx2v9Bv ULff7IjS3oRjikXFfkpJhmnPlHkNxkCPYnws+OpbaE32hi1HWz2ITruoVB7mngPP57Jd ooHgJePia4zffqy/peKGxp2dvOVHewBA6/o52hY7zHnaztwuEdych2G1nhcWRHzdA1l6 B/jVH61sdG7mshIa+bwutkg1YL5VxzAZL9PT6CGppXjT6zqUP5W6OIYqH5dow84zYi54 HINw== X-Gm-Message-State: AOJu0YxQ9O4bqtHwdjEmvCypgZdGPgN0nGc5Rdj/Ot1Dnnxsf1fEcYuv 2HCnllKUbRJKlF7nucFEM9NuGitUOMI= X-Google-Smtp-Source: AGHT+IGWm40ON9Eguf2Bke6mV/r3O3ZozH/rTAq8hwtfR0gyXwQpq+B/xX6yiiBTwIz2NA3yJgFA9A== X-Received: by 2002:a5d:58d9:0:b0:333:2fd2:51ff with SMTP id o25-20020a5d58d9000000b003332fd251ffmr2350082wrf.120.1701583028225; Sat, 02 Dec 2023 21:57:08 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id b15-20020a5d4b8f000000b003333a6b9b27sm3045465wrt.106.2023.12.02.21.57.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 02 Dec 2023 21:57:08 -0800 (PST) Message-ID: <89501b366ff0476c1b3d36ff9b6b7c80fa6fc98f.1701583024.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sun, 03 Dec 2023 05:57:04 +0000 Subject: [PATCH v3 4/4] completion: avoid user confusion in non-cone mode Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , SZEDER =?utf-8?b?R8OhYm9y?= , Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren It is tempting to think of "files and directories" of the current directory as valid inputs to the add and set subcommands of git sparse-checkout. However, in non-cone mode, they often aren't and using them as potential completions leads to *many* forms of confusion: Issue #1. It provides the *wrong* files and directories. For git sparse-checkout add we always want to add files and directories not currently in our sparse checkout, which means we want file and directories not currently present in the current working tree. Providing the files and directories currently present is thus always wrong. For git sparse-checkout set we have a similar problem except in the subset of cases where we are trying to narrow our checkout to a strict subset of what we already have. That is not a very common scenario, especially since it often does not even happen to be true for the first use of the command; for years we required users to create a sparse-checkout via git sparse-checkout init git sparse-checkout set (or use a clone option that did the init step for you at clone time). The init command creates a minimal sparse-checkout with just the top-level directory present, meaning the set command has to be used to expand the checkout. Thus, only in a special and perhaps unusual cases would any of the suggestions from normal file and directory completion be appropriate. Issue #2: Suggesting patterns that lead to warnings is unfriendly. If the user specifies any regular file and omits the leading '/', then the sparse-checkout command will warn the user that their command is problematic and suggest they use a leading slash instead. Issue #3: Completion gets confused by leading '/', and provides wrong paths. Users often want to anchor their patterns to the toplevel of the repository, especially when listing individual files. There are a number of reasons for this, but notably even sparse-checkout encourages them to do so (as noted above). However, if users do so (via adding a leading '/' to their pattern), then bash completion will interpret the leading slash not as a request for a path at the toplevel of the repository, but as a request for a path at the root of the filesytem. That means at best that completion cannot help with such paths, and if it does find any completions, they are almost guaranteed to be wrong. Issue #4: Suggesting invalid patterns from subdirectories is unfriendly. There is no per-directory equivalent to .gitignore with sparse-checkouts. There is only a single worktree-global $GIT_DIR/info/sparse-checkout file. As such, paths to files must be specified relative to the toplevel of a repository. Providing suggestions of paths that are relative to the current working directory, as bash completion defaults to, is wrong when the current working directory is not the worktree toplevel directory. Issue #5: Paths with special characters will be interpreted incorrectly The entries in the sparse-checkout file are patterns, not paths. While most paths also qualify as patterns (though even in such cases it would be better for users to not use them directly but prefix them with a leading '/'), there are a variety of special characters that would need special escaping beyond the normal shell escaping: '*', '?', '\', '[', ']', and any leading '#' or '!'. If completion suggests any such paths, users will likely expect them to be treated as an exact path rather than as a pattern that might match some number of files other than 1. However, despite the first four issues, we can note that _if_ users are using tab completion, then they are probably trying to specify a path in the index. As such, we transform their argument into a top-level-rooted pattern that matches such a file. For example, if they type: git sparse-checkout add Make we could "complete" to git sparse-checkout add /Makefile or, if they ran from the Documentation/technical/ subdirectory: git sparse-checkout add m we could "complete" it to: git sparse-checkout add /Documentation/technical/multi-pack-index.txt Note in both cases I use "complete" in quotes, because we actually add characters both before and after the argument in question, so we are kind of abusing "bash completions" to be "bash completions AND beginnings". The fifth issue is a bit stickier, especially when you consider that we not only need to deal with escaping issues because of special meanings of patterns in sparse-checkout & gitignore files, but also that we need to consider escaping issues due to ls-files needing to sometimes quote or escape characters, and because the shell needs to escape some characters. The multiple interacting forms of escaping could get ugly; this patch makes no attempt to do so and simply documents that we decided to not deal with those corner cases for now but at least get the common cases right. Signed-off-by: Elijah Newren --- contrib/completion/git-completion.bash | 89 ++++++++++++++++++++++++++ t/t9902-completion.sh | 9 ++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c614e5d4f07..185b47d802f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -3113,6 +3113,93 @@ __gitcomp_directories () fi } +# In non-cone mode, the arguments to {set,add} are supposed to be +# patterns, relative to the toplevel directory. These can be any kind +# of general pattern, like 'subdir/*.c' and we can't complete on all +# of those. However, if the user presses Tab to get tab completion, we +# presume that they are trying to provide a pattern that names a specific +# path. +__gitcomp_slash_leading_paths () +{ + local dequoted_word pfx="" cur_ toplevel + + # Since we are dealing with a sparse-checkout, subdirectories may not + # exist in the local working copy. Therefore, we want to run all + # ls-files commands relative to the repository toplevel. + toplevel="$(git rev-parse --show-toplevel)/" + + __git_dequote "$cur" + + # If the paths provided by the user already start with '/', then + # they are considered relative to the toplevel of the repository + # already. If they do not start with /, then we need to adjust + # them to start with the appropriate prefix. + case "$cur" in + /*) + cur="${cur:1}" + ;; + *) + pfx="$(__git rev-parse --show-prefix)" + esac + + # Since sparse-index is limited to cone-mode, in non-cone-mode the + # list of valid paths is precisely the cached files in the index. + # + # NEEDSWORK: + # 1) We probably need to take care of cases where ls-files + # responds with special quoting. + # 2) We probably need to take care of cases where ${cur} has + # some kind of special quoting. + # 3) On top of any quoting from 1 & 2, we have to provide an extra + # level of quoting for any paths that contain a '*', '?', '\', + # '[', ']', or leading '#' or '!' since those will be + # interpreted by sparse-checkout as something other than a + # literal path character. + # Since there are two types of quoting here, this might get really + # complex. For now, just punt on all of this... + completions="$(__git -C "${toplevel}" -c core.quotePath=false \ + ls-files --cached -- "${pfx}${cur}*" \ + | sed -e s%^%/% -e 's%$% %')" + # Note, above, though that we needed all of the completions to be + # prefixed with a '/', and we want to add a space so that bash + # completion will actually complete an entry and let us move on to + # the next one. + + # Return what we've found. + if test -n "$completions"; then + # We found some completions; return them + local IFS=$'\n' + COMPREPLY=($completions) + else + # Do NOT fall back to bash-style all-local-files-and-dirs + # when we find no match. Such options are worse than + # useless: + # 1. "git sparse-checkout add" needs paths that are NOT + # currently in the working copy. "git + # sparse-checkout set" does as well, except in the + # special cases when users are only trying to narrow + # their sparse checkout to a subset of what they + # already have. + # + # 2. A path like '.config' is ambiguous as to whether + # the user wants all '.config' files throughout the + # tree, or just the one under the current directory. + # It would result in a warning from the + # sparse-checkout command due to this. As such, all + # completions of paths should be prefixed with a + # '/'. + # + # 3. We don't want paths prefixed with a '/' to + # complete files in the system root directory, we + # want it to complete on files relative to the + # repository root. + # + # As such, make sure that NO completions are offered rather + # than falling back to bash's default completions. + COMPREPLY=( "" ) + fi +} + _git_sparse_checkout () { local subcommands="list init set disable add reapply" @@ -3138,6 +3225,8 @@ _git_sparse_checkout () fi if [[ "$using_cone" == "true" ]]; then __gitcomp_directories + else + __gitcomp_slash_leading_paths fi esac } diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index a7c3b4eb63a..bbd17f296e4 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -1571,7 +1571,7 @@ test_expect_success FUNNYNAMES,!CYGWIN 'cone mode sparse-checkout completes dire ) ' -test_expect_success 'non-cone mode sparse-checkout uses bash completion' ' +test_expect_success 'non-cone mode sparse-checkout gives rooted paths' ' # reset sparse-checkout repo to non-cone mode git -C sparse-checkout sparse-checkout disable && git -C sparse-checkout sparse-checkout set --no-cone && @@ -1581,7 +1581,12 @@ test_expect_success 'non-cone mode sparse-checkout uses bash completion' ' # expected to be empty since we have not configured # custom completion for non-cone mode test_completion "git sparse-checkout set f" <<-\EOF - + /folder1/0/1/t.txt + /folder1/expected + /folder1/out + /folder1/out_sorted + /folder2/0/t.txt + /folder3/t.txt EOF ) '