Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other
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
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
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
[pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other
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
---
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.
#
-
-## 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]}
-
Re: [pacman-dev] [PATCH] [bash_completion] Undeclared local vars/filenames with spaces/other
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
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
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
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 \
-
