Re: [PATCH 4/4] completion: avoid user confusion in non-cone mode

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 24, 2023, 01:19 UTC
Message-ID: <xmqqo7fk9cdt.fsf@gitster.g>
In-Reply-To: <fe8669a3f4f05c186e497f870c7e7ba9a94ac63f.1700761448.git.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  		if [[ "$using_cone" == "true" ]]; then
>  			__gitcomp_directories

Hmph, doesn't "Providing the files and directories currently present is thus always wrong." apply equally to cone mode?

Show 8 quoted lines

> +		else
> +			# NEEDSWORK: It might be useful to provide a
> +			# completion function which:
> +			#
> +			#     1. Provides completions based on
> +			#        files/directories that exist in HEAD, not
> +			#        just those currently present in the working
> +			#        tree.

This makes a lot of sense. May make even more sense with s/HEAD/index/, though.

Show 13 quoted lines

> +			#     4. Provides no completions when run from a
> +			#        subdirectory of the repository root.  (If we
> +			#        did provide file/directory completions, the
> +			#        user would just get a "please run from the
> +			#        toplevel directory" error message when they
> +			#        ran it.  *Further*, if the user did rerun
> +			#        the command from the toplevel, the
> +			#        completions we previously provided would
> +			#        likely be wrong as they'd be relative to the
> +			#        subdirectory rather than the repository
> +			#        root.  That could lead to users getting a
> +			#        nasty surprise based on trying to use a
> +			#        command we helped them create.)

Hmph, would an obvious alternative to (1) check against the HEAD (or the index) to see if the prefix string matches an entity at the current directory level, and then (2) to prefix the result of the previous step with "/$(git rev-parse --show-prefix)" work? That is something like this:

    $ cd t
    $ git sparse-checkout add help<TAB>
    ->
    $ git sparse-checkout add /t/helper/

and when the user gave the full path from the root level, do the obvious:

    $ cd t
    $ git sparse-checkout add /t/help<TAB>
    ->
    $ git sparse-checkout add /t/helper/

Another more fundamental approach to avoid "confusion" this bullet item tries to side step might be to *fix* the command that gets completed. As "git sparse-checkout --help" is marked as EXPERIMENTAL in capital letters, we should be able to say "what was traditionally known as 'add' is from now on called 'add-pattern' and command line completion would not get in the way; the 'add' subcommand now takes only literal paths, not patterns, that are relative to the current directory" if we wanted to.

Show 13 quoted lines

> +			#     5. Provides escaped completions for any paths
> +			#        containing a '*', '?', '\', '[', ']', or
> +			#        leading '#' or '!'.  (These characters might
> +			#        already be escaped to protect from the
> +			#        shell, but they need an *extra* layer of
> +			#        escaping to prevent the pattern parsing in
> +			#        Git from seeing them as special characters.)
> +			#
> +			# Of course, this would be a lot of work, so for now,
> +			# just avoid the many forms of user confusion that
> +			# could be caused by providing bad completions by
> +			# providing a fake completion to avoid falling back to
> +			# bash's normal file and directory completion.

> +			COMPREPLY=( "" )
>  		fi
>  	esac
>  }