[Bug 1397520] [NEW] tighten up completion script

[Peter Cordes]

Public bug reported:

patch against dput 0.9.6.4ubuntu1.1

1.  /usr/share/doc/bash-completion/README.Debian says to use dh_bash-
completion to install the completion script.  Then it's not even a
conffile, so you don't have to test have dput before loading the
completion funtion, since it won't be left lying around without dput
installed.  (I didn't include this in my patch, just the changes to the
dput completion script itself.)

patch attached.  Ironically, given what package I'm working on, I'm not
at all an expert in debian package rules.  I'm just hacking the dput
completion script I looked at it when investigating bug 509512, and
couldn't leave it that clunky.  Anyway, I'll leave adding dh_bash-
completion for the maintainer.

2. It uses a lot more commands than it needs to.
the hosts=$( ... )  section run a grep| tr pipeline separately for two different files.  It even runs /bin/true, rather than builtin true!

tr -d [] can be done as sed 's/[][]*//g',

The grep part can be incorporated as a match condition for the s/// operation:
sed -n '/^\[.*\]/s/[][]*//gp'

and then the grep -v can be thrown in, as well:


And sed (like grep) supports multiple input filename arguments, and doesn't abort if it has trouble reading from the first one.
hosts=$(  sed -n -e '/^\[DEFAULT\]/d' -e '/^\[.*\]/s/[][]*//gp'  ~/.dput.cf /etc/dput.cf 2>/dev/null  | sort -u)

true isn't needed, since sed isn't at the end of a pipeline, so its exit
status doesn't matter.  Not sure if sort --unique is needed, or if
bash's completion stuff will filter dups.

I think the 3 separate invocations of compgen could collapse to
compgen -f -X '!*.@(asc|changes|sig)' -- "$cur"

compgen | grep is silly.  pass the pattern as an arg to compgen, like

COMPREPLY=( $(
    compgen -f -X '!*.@(asc|changes|sig)' -- "$cur"
    compgen -W "$paroptions" -- "$cur"
                ) )

 With all these changes, it should run a LOT faster, and go from 0.0
seconds to 0.0 seconds...  yup, useful.

anyway, like I said, patch attached.  It'd be great if this could get
forwarded to the Debian maintainers.

** Affects: dput (Ubuntu)
     Importance: Undecided
         Status: New


** Tags: patch

** Attachment added: "patch for /etc/bash_completion.d/dput.  Assumes dh_bash-completion change in the package rules"
   https://bugs.launchpad.net/bugs/1397520/+attachment/4270678/+files/dput.completion.patch

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to dput in Ubuntu.
https://bugs.launchpad.net/bugs/1397520

Title:
  tighten up completion script

Status in dput package in Ubuntu:
  New

Bug description:
  patch against dput 0.9.6.4ubuntu1.1

  1.  /usr/share/doc/bash-completion/README.Debian says to use dh_bash-
  completion to install the completion script.  Then it's not even a
  conffile, so you don't have to test have dput before loading the
  completion funtion, since it won't be left lying around without dput
  installed.  (I didn't include this in my patch, just the changes to
  the dput completion script itself.)

  patch attached.  Ironically, given what package I'm working on, I'm
  not at all an expert in debian package rules.  I'm just hacking the
  dput completion script I looked at it when investigating bug 509512,
  and couldn't leave it that clunky.  Anyway, I'll leave adding dh_bash-
  completion for the maintainer.

  2. It uses a lot more commands than it needs to.
  the hosts=$( ... )  section run a grep| tr pipeline separately for two different files.  It even runs /bin/true, rather than builtin true!

  tr -d [] can be done as sed 's/[][]*//g',

  The grep part can be incorporated as a match condition for the s/// operation:
  sed -n '/^\[.*\]/s/[][]*//gp'

  and then the grep -v can be thrown in, as well:

  
  And sed (like grep) supports multiple input filename arguments, and doesn't abort if it has trouble reading from the first one.
  hosts=$(  sed -n -e '/^\[DEFAULT\]/d' -e '/^\[.*\]/s/[][]*//gp'  ~/.dput.cf /etc/dput.cf 2>/dev/null  | sort -u)

  true isn't needed, since sed isn't at the end of a pipeline, so its
  exit status doesn't matter.  Not sure if sort --unique is needed, or
  if bash's completion stuff will filter dups.

  I think the 3 separate invocations of compgen could collapse to
  compgen -f -X '!*.@(asc|changes|sig)' -- "$cur"

  compgen | grep is silly.  pass the pattern as an arg to compgen, like

  COMPREPLY=( $(
      compgen -f -X '!*.@(asc|changes|sig)' -- "$cur"
      compgen -W "$paroptions" -- "$cur"
                  ) )

   With all these changes, it should run a LOT faster, and go from 0.0
  seconds to 0.0 seconds...  yup, useful.

  anyway, like I said, patch attached.  It'd be great if this could get
  forwarded to the Debian maintainers.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/dput/+bug/1397520/+subscriptions