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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,


On Thu, Oct 14, 2010 at 12:15:07PM -0500, Jonathan Nieder wrote:
> 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?).

> > @@ -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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]