Re: [PATCH v3 1/2] completion: refactor existence checks for pseudorefs
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Jan 13, 2024, 21:07 UTC
- Message-ID
- <20240113210714.GC3000857@szeder.dev>
- In-Reply-To
- <29c928649aba7dfce4dab1b5d923bc23b7af2d32.1703022850.git.stanhu@gmail.com>
On Tue, Dec 19, 2023 at 02:14:17PM -0800, Stan Hu wrote:
Show 24 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..8edd002eed 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 pseudoref exists.
> +# 1: The pseudo-ref to search
> +__git_pseudoref_exists ()
> +{
> + local ref=$1
> +
> + [ -f "$__git_repo_path/$ref" ]This new helper function relies on $__git_repo_path being set, but it doesn't ensure that it's actually set by calling __git_find_repo_path. Instead, it relies on its callers calling __git_find_repo_path, although after this patch none of those callers directly access $__git_repo_path anymore.
It would be better to call __git_find_repo_path in this helper to make it more self-contained, and then the now unnecessary __git_find_repo_path calls from the three callers can be removed.
Yeah, I know it's late, because this patch is already in master, but this would be a good preparation step for eventual dedicated tests of __git_pseudoref_exists that came up in:
https://public-inbox.org/git/20240113191749.GB3000857@szeder.dev/
Show 62 quoted lines
> +}
> +
> # Removes backslash escaping, single quotes and double quotes from a word,
> # stores the result in the variable $dequoted_word.
> # 1: The word to dequote.
> @@ -1625,7 +1634,7 @@ __git_cherry_pick_inprogress_options=$__git_sequencer_inprogress_options
> _git_cherry_pick ()
> {
> __git_find_repo_path
> - if [ -f "$__git_repo_path"/CHERRY_PICK_HEAD ]; then
> + if __git_pseudoref_exists CHERRY_PICK_HEAD; then
> __gitcomp "$__git_cherry_pick_inprogress_options"
> return
> fi
> @@ -2067,7 +2076,7 @@ _git_log ()
> __git_find_repo_path
>
> local merge=""
> - if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> + if __git_pseudoref_exists MERGE_HEAD; then
> merge="--merge"
> fi
> case "$prev,$cur" in
> @@ -2934,6 +2943,7 @@ _git_reset ()
>
> _git_restore ()
> {
> + __git_find_repo_path
> case "$prev" in
> -s)
> __git_complete_refs
> @@ -2952,7 +2962,7 @@ _git_restore ()
> __gitcomp_builtin restore
> ;;
> *)
> - if __git rev-parse --verify --quiet HEAD >/dev/null; then
> + if __git_pseudoref_exists HEAD; then
> __git_complete_index_file "--modified"
> fi
> esac
> @@ -2963,7 +2973,7 @@ __git_revert_inprogress_options=$__git_sequencer_inprogress_options
> _git_revert ()
> {
> __git_find_repo_path
> - if [ -f "$__git_repo_path"/REVERT_HEAD ]; then
> + if __git_pseudoref_exists REVERT_HEAD; then
> __gitcomp "$__git_revert_inprogress_options"
> return
> fi
> @@ -3592,7 +3602,7 @@ __gitk_main ()
> __git_find_repo_path
>
> local merge=""
> - if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> + if __git_pseudoref_exists MERGE_HEAD; then
> merge="--merge"
> fi
> case "$cur" in
> --
> 2.42.0
>
>