Re: [PATCH v2 1/2] completion: refactor existence checks for pseudorefs

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 8, 2023, 08:24 UTC
Message-ID: <ZXLSpyE5ZW0ANmsi@tanuki>
In-Reply-To: <1c6a747691f36ede4224b6d4c2e0c8fd4c0575fd.1701928891.git.stanhu@gmail.com>

On Wed, Dec 06, 2023 at 10:06:39PM -0800, Stan Hu wrote:

Show 20 quoted lines

> In preparation for the reftable backend, this commit introduces a
> '__git_pseudoref_exists' function that continues to use 'test -f' to
> determine whether a given pseudoref exists in the local filesystem.
> 
> Signed-off-by: Stan Hu <stanhu@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 13a39ebd2e..9fbdc13f9a 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -122,6 +122,15 @@ __git ()
>  		${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null
>  }
>  
> +# Runs git in $__git_repo_path to determine whether a ref exists.
> +# 1: The ref to search
> +__git_ref_exists ()

I first thought that you missed Junio's point that `__git_ref_exists` may better be renamed to something lkie `__git_pseudoref_exists`. But you do indeed change the name in the second patch. I'd propose to instead squash the rename into the first patch so that the series becomes easier to read.

Patrick