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