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

Sami Kerola
2013-04-01T16:52:51Z

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