Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-05-09 Thread Andres P
On Sat, May 8, 2010 at 10:31 AM, Xavier Chantry shinin...@gmail.com 

wrote:



 Attached list of changes between maint and master, + the new patch
 rebased on top of master.

 Of course if you want to keep working on it, you're very welcome to 

do so :)



What mostly annoyed me with pacman bash completion is the lag/delay on
completing packages in pacman -S.
That clearly feels better after your patch, but I've noticed a funny
problem and different behavior.

I am sitting in my pacman git tree, and wanted to reinstall pacman :
$ pacman -S pactab
after your patch :
$ sudo pacman -S pac
pacbuild   pacman-contrib pacman-mirrorlist  paco
 pacpan pacupdate
before :
$ sudo pacman -S pac
pacbuild   pacman/pacman-contrib
pacman-mirrorlist  paco   pacpan pacupdate

So actually I couldnt complete pacman with your patch, it proposed me
pacman- directly.
The old behavior didnt propose me pacman either, but since it
hesitated between pacman/ and pacman- , the completion stopped on
pacman which is the good package name :)

And this weird behavior is because I had a pacman dir in my PWD :
$ ls -ld pacman/
drwxr-xr-x 2 xavier users 4096 Sep 23  2009 pacman/


Yeah, that was because of the very last lines on my old patch.

I had something to the effect of:

- complete -o filenames -o plusdirs -F _pacman pacman
+ complete -o filenames -F _pacman pacman

I skipped plusdirs in my new patch so this is now fixed,
Andres P



Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-05-08 Thread Xavier Chantry
On Wed, Jan 20, 2010 at 9:27 AM, Andres Perera andres...@gmail.com wrote:
 Uh, I broke the spaces with git's 72 textwidth.

 Same patch attached...





I don't understand what you mean, this is the diff between original
and new patch :

diff --git a/contrib/bash_completion b/contrib/bash_completion
index 2f6cd06..b1162ad 100644
--- a/contrib/bash_completion
+++ b/contrib/bash_completion
@@ -1,6 +1,6 @@
 # pacman/makepkg completion by Andres Perera andres87p gmail
 #
-# Distributed under the terms of the GNU General Public License v3 or
+# Distributed under the terms of the GNU General Public License v2 or
 # later.
 #
 #   Local variables: common core cur glob list long m o prev query r
@@ -56,7 +56,7 @@ _makepkg_count_words() {
 _makepkg() {
   COMPREPLY=()

-  local prev cur short long parse glob
+  local prev cur short long
   prev=${COMP_WORDS[COMP_CWORD-1]}
   cur=`_get_cword`


I see you reverted the license and made another change, but no space change.
I am not sure what you mean with git's 72 textwidth , isnt that just
commit log ? Your original commit log was fine.
Also it's much easier for us if you keep submitting git patches with the log.
And finally the patch now only applies against maint, not master, but
maybe that's our fault of not applying it earlier :)



Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-05-08 Thread Andres P
On 06/08/10 at 09:54:16 CEST, Xavier Chantry shiningxc at gmail.com 
wrote:
On Wed, Jan 20, 2010 at 9:27 AM, Andres Perera andres...@gmail.com 

wrote:

 Uh, I broke the spaces with git's 72 textwidth.

 Same patch attached...





I don't understand what you mean, this is the diff between original
and new patch :

diff --git a/contrib/bash_completion b/contrib/bash_completion
index 2f6cd06..b1162ad 100644
--- a/contrib/bash_completion
+++ b/contrib/bash_completion
@@ -1,6 +1,6 @@
 # pacman/makepkg completion by Andres Perera andres87p gmail
 #
-# Distributed under the terms of the GNU General Public License v3 or
+# Distributed under the terms of the GNU General Public License v2 or
 # later.
 #
 #   Local variables: common core cur glob list long m o prev query r
@@ -56,7 +56,7 @@ _makepkg_count_words() {
 _makepkg() {
   COMPREPLY=()

-  local prev cur short long parse glob
+  local prev cur short long
   prev=${COMP_WORDS[COMP_CWORD-1]}
   cur=`_get_cword`

Oh, I was under the impression I broke spaces. Maybe it was the gmail 
client

playing tricks on me

I see you reverted the license and made another change, but no space 

change.

I am not sure what you mean with git's 72 textwidth , isnt that just
commit log ? Your original commit log was fine.
Also it's much easier for us if you keep submitting git patches with  

the log.

And finally the patch now only applies against maint, not master, but
maybe that's our fault of not applying it earlier :)

To be honest I'm not really satisfied with that patch anymore, since it 
could
be made even more simple and faster. I also had put an option that made 
dirs

show twice (+plusdirs), ie a new bug.

I'll try and see if I can get something clean against master,
Andres



Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-01-19 Thread Dan McGee
On Tue, Jan 19, 2010 at 11:26 PM, Andres Perera andres...@gmail.com wrote:
 Fix:
 3 Undeclared local vars with common enough names to warrant breakage

 Performance issues with _pacman trying to replicate /usr/bin/pacman
 with find and other slow tools.

 Performance issues with expanding an array (with sometimes hundreds of
 items) over three times.

 Expanding said array to remove already completed entries had the side
 effect of braking filenames with spaces and or \n.

 The full description of fixes are already posted at:
 http://bugs.archlinux.org/task/16630#comment55779
 along with time diffs.

 Signed-off-by: Andres P andres...@gmail.com
 ---
First off, thanks. These fixes sound much needed.

  contrib/bash_completion |  535 
 ++-
  1 files changed, 202 insertions(+), 333 deletions(-)

 diff --git a/contrib/bash_completion b/contrib/bash_completion
 index 62e5bc9..65051f5 100644
 --- a/contrib/bash_completion
 +++ b/contrib/bash_completion
 @@ -1,365 +1,234 @@
 -# vim: set ft=sh ts=2 sw=2 et:
 -# file: /etc/bash_completion.d/pacman
 -
 -# Bash completion for pacman
 -# Original: Manolis Tzanidakis mtzanida...@freemail.gr
 +# pacman/makepkg completion by Andres Perera andres87p gmail
  #
 -# Distributed under the terms of the GNU General Public License, v2 or later.
 +# Distributed under the terms of the GNU General Public License v3 or
 +# later.
Is this strictly necessary? You are changing one piece of code in the
entire codebase to require v3 or later and as weird as it sounds, I am
not going to let that fly. I won't be merging this unless it is v2 or
you give some darn good reasons why we should move to v3.

For the rest of the patch, I think I am just going to test it out
locally and then I'll get back to you if I don't see anything that
blows up as you are right in it being a total revision of the
original.

-Dan



Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-01-19 Thread Andres Perera
On Wed, Jan 20, 2010 at 1:07 AM, Dan McGee dpmc...@gmail.com wrote:
 Is this strictly necessary? You are changing one piece of code in the
 entire codebase to require v3 or later and as weird as it sounds, I am
 not going to let that fly. I won't be merging this unless it is v2 or
 you give some darn good reasons why we should move to v3.

Not really necessary. You can change it to v2, or I can do that if it
requires a commit from me.



Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-01-19 Thread Dan McGee
On Tue, Jan 19, 2010 at 11:48 PM, Andres Perera andres...@gmail.com wrote:
 On Wed, Jan 20, 2010 at 1:07 AM, Dan McGee dpmc...@gmail.com wrote:
 Is this strictly necessary? You are changing one piece of code in the
 entire codebase to require v3 or later and as weird as it sounds, I am
 not going to let that fly. I won't be merging this unless it is v2 or
 you give some darn good reasons why we should move to v3.

 Not really necessary. You can change it to v2, or I can do that if it
 requires a commit from me.

Nah that's good enough for me. I'll try to give this whole thing a
workout in the next day or two and see if I can come up with anything
else that we might want to tweak, however.

-Dan



Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other

2010-01-19 Thread Andres Perera
Uh, I broke the spaces with git's 72 textwidth.

Same patch attached...
diff --git a/contrib/bash_completion b/contrib/bash_completion
index 62e5bc9..218321a 100644
--- a/contrib/bash_completion
+++ b/contrib/bash_completion
@@ -1,365 +1,234 @@
-# vim: set ft=sh ts=2 sw=2 et:
-# file: /etc/bash_completion.d/pacman
-
-# Bash completion for pacman
-# Original: Manolis Tzanidakis mtzanida...@freemail.gr
+# pacman/makepkg completion by Andres Perera andres87p gmail
 #
-# Distributed under the terms of the GNU General Public License, v2 or later.
+# Distributed under the terms of the GNU General Public License v2 or
+# later.
 #
-
-## initial functions
-
-rem_selected ()
-{
-  # (Adapted from bash_completion by Ian Macdonald i...@caliban.org)
-  # This removes any options from the list of completions that have
-  # already been specified on the command line.
-  COMPREPLY=($(/bin/echo ${comp_wor...@]} | \
-(while read -d ' ' i; do
-  [ ${i} ==  ]  continue
-  # flatten array with spaces on either side,
-  # otherwise we cannot grep on word boundaries of
-  # first and last word
-  COMPREPLY= ${comprep...@]} 
-  # remove word from list of completions
-  COMPREPLY=(${COMPREPLY/ ${i%% *} / })
+#   Local variables: common core cur glob list long m o prev query r
+#remove s short sync upgrade w
+#
+# vim: ft=sh sts=2 sw=2 et:
+
+# Removes packages/files already present in the line
+_arch_rem_selected() {
+  local w r
+
+  for (( w=0; w${#comp_wor...@]}-1; w++)); do
+# Don't touch operands
+[[ ${COMP_WORDS[w]} == -* ]]  continue
+for r in ${!comprep...@]}; do
+  # Only deal with full matches
+  if [[ ${COMP_WORDS[w]} == ${COMPREPLY[r]} ]]; then
+unset 'COMPREPLY[r]'; break
+  fi
 done
-/bin/echo ${comprep...@]})))
-  return 0
-}
-
-_available_repos ()
-{
-  COMPREPLY=( $( compgen -W $(/bin/grep '\[' /etc/pacman.conf | /bin/grep -v 
-e 'options' -e '^#' | tr -d '[]' ) -- $cur ) )
-}
-
-_installed_pkgs ()
-{
-  local installed_pkgs
-  installed_pkgs=$( /bin/ls /var/lib/pacman/local/ )
-  COMPREPLY=( $( compgen -W $( for i in $installed_pkgs; do /bin/echo 
${i%-*-*}; done ) -- $cur ) )
+  done
 }
 
-_available_pkgs ()
-{
-  #find balks easilly on a find /foo/*/* type dir, especially one like
-  #   /var/lib/pacman/*/*
-  # This little change-up removes the find *and* only uses enabled repos
-  local available_pkgs
-  local enabled_repos
-  enabled_repos=$( /bin/grep '\[' /etc/pacman.conf | /bin/grep -v -e 'options' 
-e '^#' | tr -d '[]' )
-  available_pkgs=$( for r in $enabled_repos; do /bin/echo 
/var/lib/pacman/sync/$r/*; done )
-  COMPREPLY=( $( compgen -W $( for i in $available_pkgs; do j=${i##*/}; echo 
${j%-*-*}; done ) -- $cur ) )
-}
+# makepkg purge functions
+_makepkg_deselect() {
+  local o
 
-_installed_groups ()
-{
-  local installed_groups
-  installed_groups=$( /bin/find /var/lib/pacman/local -name desc -exec 
/bin/sed -ne '/%GROUPS%/,/^$/{//d; p}' {} \; | /bin/sort -u )
-  COMPREPLY=( $( compgen -W $( for i in $installed_groups; do /bin/echo 
${i%-*-*}; done ) -- $cur ) )
+  if [[ $1 == l ]]; then
+for o in ${!lo...@]}; do
+  if [[ ${COMP_WORDS[w]} == ${long[o]} ]]; then
+unset 'long[o]'; break
+  fi
+done
+  else
+for o in ${!sho...@]}; do
+  if [[ ${COMP_WORDS[w]} == -*([^- ])${short[o]}* ]]; then
+unset 'short[o]'
+  fi
+done
+  fi
 }
 
-_available_groups ()
-{
-  #find balks easilly on a find /foo/*/* type dir, especially one like
-  #   /var/lib/pacman/*/*
-  # This little change-up removes the find *and* only uses enabled repos
-  local available_groups
-  local enabled_repos
-  enabled_repos=$( /bin/grep '\[' /etc/pacman.conf | /bin/grep -v -e 'options' 
-e '^#' | tr -d '[]' )
-  available_groups=$( for r in $enabled_repos; do /bin/sed 
'/%GROUPS%/,/^$/{//d; p}' /var/lib/pacman/sync/$r/*/desc | /bin/sort -u; done )
-  COMPREPLY=( $( compgen -W $( for i in $available_groups; do /bin/echo 
${i%-*-*}; done ) -- $cur ) )
+_makepkg_count_words() {
+  local w
+  
+  for (( w=0; w${#comp_wor...@]}; w++)); do
+[[ ${COMP_WORDS[w]} == @(-*([^- ])[hC]*|--@(help|cleancache)) ]]  return 0
+_makepkg_deselect $1
+  done
 }
 
-## makepkg completion
-
-_makepkg ()
-{
-  local cur prev
+# makepkg main
+_makepkg() {
   COMPREPLY=()
-  cur=${COMP_WORDS[COMP_CWORD]}
-  prev=${COMP_WORDS[COMP_CWORD-1]}
 
-  case $prev in
--p)
-  _filedir
-  return 0
-;;
---help|--cleancache)
-  COMPREPLY=''
-  return 0
-;;
-  esac
-
-  if [[ $cur == -* ]]; then
-COMPREPLY=( $( compgen -W '\
-  -A --ignorearch \
-  -b --builddeps \
-  -c --clean \
-  -C --cleancache \
-  -d --nodeps \
-  -e --noextract \
-  -f --force \
-  -g --geninteg \
-  -h --help \
-  -i --install \
-  -L --log \
-  -m --nocolor \
-  -o --nobuild \
-  -p \
-  -r --rmdeps \
-  -s --syncdeps \
-  --asroot \
-