Re: [PATCH 1/2] bash: add helper function to get config variables for completion

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Oct 20, 2010, 22:29 UTC
Message-ID: <20101020222949.GA1767@neumann>
In-Reply-To: <20101014171507.GA17550@burratino>

Hi Jonathan,

On Thu, Oct 14, 2010 at 12:15:07PM -0500, Jonathan Nieder wrote:

Show 24 quoted lines

> SZEDER Gábor wrote:
> 
> > Currently there are three completion functions that perform similar
> > queries to 'git config' to get config variable names.
> 
> Good point.
> 
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -451,10 +451,7 @@ __git_remotes ()
> >  		echo ${i#$d/remotes/}
> >  	done
> >  	[ "$ngoff" ] && shopt -u nullglob
> > -	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
> > -		i="${i#remote.}"
> > -		echo "${i/.url*/}"
> > -	done
> > +	__git_get_config_variables "remote" "url"
> >  }
> 
> Ok, so __git_get_config_variables $category $var means something like
> 
> 	git config --get-regexp '$category[.].*[.]$var' |
> 	cut -d. -f2

Almost. Considering the current invocations of __git_get_config_variables() introduced in this patch, yes, they do the same. But "cut -d. -f2" will behave differently when $category contains a dot, or when neither $category nor $var contain a dot, but the config variable contains more than two (does git have any such config variables?).

Show 36 quoted lines

> > @@ -750,14 +747,16 @@ __git_compute_porcelain_commands ()
> >  	: ${__git_porcelain_commands:=$(__git_list_porcelain_commands)}
> >  }
> >  
> > -__git_aliases ()
> > +# returns all config variables within a given section with an optional
> > +# suffix, with both the section name and the suffix removed
> > +__git_get_config_variables ()
> >  {
> > -	local i IFS=$'\n'
> > -	for i in $(git --git-dir="$(__gitdir)" config --get-regexp "alias\..*" 2>/dev/null); do
> > +	local section="$1" suffix="${2-}" i IFS=$'\n'
> > +	for i in $(git --git-dir="$(__gitdir)" config --get-regexp "$section\..*${suffix:+\.$suffix}" 2>/dev/null); do
> 
> Would it be possible to shorten this line?  e.g.
> 
> 	for i in $(
> 		git --git-dir="$(__gitdir)" ...
> 	); do
> 
> or
> 
> 	while read -r setting
> 	do
> 		...
> 	done < <(
> 		git --git-dir="$(__gitdir)" ...
> 	)
> 
> or
> 
> 	local ... IFS=$'\n'
> 	set -- $(git ... )
> 	for i do
> 		...
> 	done

Well, yes, of course. But the original line was already too long, and neither of your proposals in itself would make it short enough to fit 80 characters. Besides, the latter two changes the loop itself, not just what the body of the loop does and what it is looping on.

Maybe we could just split the line in two in the middle, like

	for i in $(git --git-dir="$(__gitdir)" config --get-regexp \
		$section\..*${suffix:+\.$suffix}" 2>/dev/null); do

Still doesn't look pretty, but maybe a bit better.

Best, Gábor