From: Junio C Hamano Date: Mon, 14 May 2012 18:13:01 GMT Subject: Re: [PATCH v5] completion: add new __git_complete helper Message-ID: <7vmx5ahbrm.fsf@alter.siamese.dyndns.org> In-Reply-To: Felipe Contreras writes: >>> +__git_func_wrap () >>> +{ >>> +     if [[ -n ${ZSH_VERSION-} ]]; then >>> +             emulate -L bash >>> +             setopt KSH_TYPESET >>> + >>> +             # workaround zsh's bug that leaves 'words' as a special >>> +             # variable in versions < 4.3.12 >>> +             typeset -h words >>> + >>> +             # workaround zsh's bug that quotes spaces in the COMPREPLY >>> +             # array if IFS doesn't contain spaces. >>> +             typeset -h IFS >>> +     fi >>> +     local cur words cword prev >>> +     _get_comp_words_by_ref -n =: cur words cword prev >>> +     $1 >>> +} >>> + >>> +# Setup completion for certain functions defined above by setting common >>> +# variables and workarounds. >>> +# This is NOT a public function; use at your own risk. >>> +__git_complete () >>> +{ >>> +     local wrapper="__git_wrap${2}" >>> +     eval "$wrapper () { __git_func_wrap $2 ; }" >>> +     complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \ >>> +             || complete -o default -o nospace -F $wrapper $1 >>> +} >>> + >>> +__git_complete git _git >>> +__git_complete gitk _gitk >> >> It makes my stomach queasy whenever I see $var not in double quotes that >> forces me to guess (and trace to verify if the codepath is what I really >> care about) if any value with $IFS in it could be used there, so even when >> they are known to be safe, I'd prefer to see either explicit quotes or >> comment that says what are expected in $1 and $2. > > Which ones? All of them ;-) Here is my attempt to explain why none of them needs to be quoted: # Setup completion for certain functions defined above by setting common # variables and workarounds. # It takes two parameters: # - the first is the command name on the command line to complete its # arguments for the user; # - the second is a name of the completion function # This is NOT a public function; use at your own risk. # # Note that none of the variable reference in the implementation of this # function needs dq around it. # # wrapper: the name of an internal shell function that wraps the # completion function $2, formed by prefixing "__git_wrap" # in front of it. As it has to be usable as a name of a # shell function, by definition there won't be $IFS characters # in it. # $1: the command name on the command line---ditto. # $2: the shell function name that implements the completion-ditto. Once there is such an explanation, the answer to the next question would become... >>> +     $1 >>> +     complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \ >>> +             || complete -o default -o nospace -F $wrapper $1 > > So you want: > > "$1" ... Not really.