Re: [PATCH 05/10] bash-completion: misc-utils

[Sami Kerola <kerolasa@iki.fi>]

On Wed, Mar 27, 2013 at 09:42:41PM -0400, Dave Reisner wrote:
> On Wed, Mar 27, 2013 at 10:07:47PM +0000, Sami Kerola wrote:
> > +	local PARTITIONS
> > +	PARTITIONS="$(for I in /sys/block/*/*/partition; do IFS='/'; DIR_ARR=($I); echo "/dev/${DIR_ARR[4]}"; done)"
> 
> You want the loop *outside* the assignment, rather than exploding the
> contents into the array via echo:
> 
>   for part in /sys/block/*/*/partition; do
>     IFS=/ read -ra a <<< "$part"
>     partitions+=("${a[4]}")
>   done

That was changed to use 'lsblk' rather than /sys files.

> > +		'-N'|'--task')
> > +			local TID I
> > +			TID="$(for I in /proc/*/mountinfo; do IFS='/'; TID=($I); echo "${TID[2]}"; done)"
> 
> Ditto here

Corrected.

> > +			COMPREPLY=( $(compgen -W "$TID" -- $cur) )
> > +			return 0
> > +			;;
> > +		'-O'|'--options')
> > +			local MNT_OPTS
> > +			MNT_OPTS=$(awk '{ n = split($4, a, ","); for (i = 0; i <= n; i++) { mnt_opts[a[i]]=1 } } END { for (i in mnt_opts) { print i } }' /etc/mtab 2>/dev/null )
> 
> I'm not sure you want to get into this game. You're including values
> from key=val pairs (e.g. the 300 in timeout=300). Regardless, no awk
> required here
> 
>   declare -A mnt_opts
> 
>   while read _ _ _ optstring _; do
>     IFS=, read -ra opts <<< "$optstring"
>     for opt in "${opts[@]}"; do
>       mnt_opts["${opt%%=*}"]=1
>     done
>   done </etc/mtab
> 
>   opts=("${!mnt_opts[@]}")

Something similar to that was done with a small difference to avoid
reading /etc/mtab.  Notice that findmnt is using key=val pair when
matching with mount options, not key only.

> > +			COMPREPLY=( $(compgen -W "$MNT_OPTS" -- $cur) )
> > +			return 0
> > +			;;
> > +		'-o'|'--output')
> > +			# FIXME: how to append to a string with compgen?
> 
> I've been down this road when I wrote some of the systemd completion. I
> don't think you want to go there.

I am not sure if CSV filling with compgen is currently possible.  I have
a feeling the whole issue would be easier if after a completetion which
resulted to nospace a word break would be inserted.  Is that missing bash
feature, bug, or my misunderstanding how this case would be nicest to
solve.  While I do not know belive leaving the FIXME items in place is
correct thing to do.

Thanks for reviews Dave.

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/