New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aura completions #1292

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@mrak
Contributor

mrak commented Feb 7, 2014

This merge has two things:

  1. aura completions. (project page)
  2. pacman integration for __fish_print_packages
@fosskers

This comment has been minimized.

Show comment
Hide comment
@fosskers

fosskers Feb 7, 2014

Contributor

@mrak Thank you!

Contributor

fosskers commented Feb 7, 2014

@mrak Thank you!

@mrak

This comment has been minimized.

Show comment
Hide comment
@mrak

mrak Feb 7, 2014

Contributor

@fosskers
I was planning on referencing this PR in the enhancement issue you have in aura once it was merged.

I definitely make heavy use of both fish and aura :-)

Contributor

mrak commented Feb 7, 2014

@fosskers
I was planning on referencing this PR in the enhancement issue you have in aura once it was merged.

I definitely make heavy use of both fish and aura :-)

@fosskers

This comment has been minimized.

Show comment
Hide comment
@fosskers

fosskers Feb 7, 2014

Contributor

I watch this project so saw this pull request in my notifications the instant you made it, haha.

Contributor

fosskers commented Feb 7, 2014

I watch this project so saw this pull request in my notifications the instant you made it, haha.

@mrak mrak referenced this pull request Feb 7, 2014

Closed

Fish Completions #87

@zanchey

View changes

Show outdated Hide outdated share/functions/__fish_print_packages.fish
@@ -70,6 +70,12 @@ function __fish_print_packages
rpm -qa |sed -e 's/-[^-]*-[^-]*$/\t'$package'/' >$cache_file &
end
# This completes pacman package names from pacman's cache
if type -f pacman >/dev/null
pacman -Sl | cut -d ' ' -f 2- | tr ' ' \t

This comment has been minimized.

@zanchey

zanchey Feb 11, 2014

Member

How fast is pacman -Sl? For comparison, rpm -qa takes about 900 msec hot and cold, apt-cache pkgnames is more like 40 msec cold and less when hot.

@zanchey

zanchey Feb 11, 2014

Member

How fast is pacman -Sl? For comparison, rpm -qa takes about 900 msec hot and cold, apt-cache pkgnames is more like 40 msec cold and less when hot.

This comment has been minimized.

@terlar

terlar Feb 11, 2014

Contributor
real 0.44
user 0.16
sys 0.02
@terlar

terlar Feb 11, 2014

Contributor
real 0.44
user 0.16
sys 0.02

This comment has been minimized.

@zanchey

zanchey Feb 11, 2014

Member

Yikes. Is that hot and cold? 400 msec is probably above the cutoff for interactive use, which I would peg about 200 msec. Perhaps caching it in the same manner as rpm is wise.

@zanchey

zanchey Feb 11, 2014

Member

Yikes. Is that hot and cold? 400 msec is probably above the cutoff for interactive use, which I would peg about 200 msec. Perhaps caching it in the same manner as rpm is wise.

This comment has been minimized.

@terlar

terlar Feb 11, 2014

Contributor

That was hot, I think the cold was about the same. It is not super slow, but for speedy feeling it might be a good idea to cache it.

Edit:

➥ time -p pacman -Sl >/dev/null
real 0.22
user 0.17
sys 0.00

If I skipped the output, it was much faster for real, but still...

@terlar

terlar Feb 11, 2014

Contributor

That was hot, I think the cold was about the same. It is not super slow, but for speedy feeling it might be a good idea to cache it.

Edit:

➥ time -p pacman -Sl >/dev/null
real 0.22
user 0.17
sys 0.00

If I skipped the output, it was much faster for real, but still...

This comment has been minimized.

@fosskers

fosskers Feb 11, 2014

Contributor

If all he wants are package names, pacman -Ssq should be faster.

@fosskers

fosskers Feb 11, 2014

Contributor

If all he wants are package names, pacman -Ssq should be faster.

This comment has been minimized.

@fosskers

fosskers Feb 12, 2014

Contributor
colin@ko-linux ~> time pacman -Ssq > /dev/null
0.22user 0.02system 0:00.31elapsed 79%CPU (0avgtext+0avgdata 15176maxresident)k
0inputs+0outputs (0major+4803minor)pagefaults 0swaps

Compared to:

colin@ko-linux ~> time pacman -Sl > /dev/null
0.26user 0.00system 0:00.34elapsed 80%CPU (0avgtext+0avgdata 15600maxresident)k
0inputs+0outputs (0major+4943minor)pagefaults 0swaps
@fosskers

fosskers Feb 12, 2014

Contributor
colin@ko-linux ~> time pacman -Ssq > /dev/null
0.22user 0.02system 0:00.31elapsed 79%CPU (0avgtext+0avgdata 15176maxresident)k
0inputs+0outputs (0major+4803minor)pagefaults 0swaps

Compared to:

colin@ko-linux ~> time pacman -Sl > /dev/null
0.26user 0.00system 0:00.34elapsed 80%CPU (0avgtext+0avgdata 15600maxresident)k
0inputs+0outputs (0major+4943minor)pagefaults 0swaps

This comment has been minimized.

@mrak

mrak Feb 12, 2014

Contributor
time pacman -Ssq | sed -e 's/$/\tPackage/' >/dev/null
pacman -Ssq  0.15s user 0.01s system 99% cpu 0.158 total
sed -e 's/$/\tPackage/' > /dev/null  0.01s user 0.00s system 4% cpu 0.158 total
time pacman -Ssq >/dev/null                          
pacman -Ssq > /dev/null  0.15s user 0.00s system 99% cpu 0.154 total
time pacman -Slq | sed -e 's/$/\tPackage/' >/dev/null 
pacman -Slq  0.16s user 0.00s system 92% cpu 0.172 total
sed -e 's/$/\tPackage/' > /dev/null  0.03s user 0.00s system 17% cpu 0.172 total
time pacman -Slq >/dev/null                          
pacman -Slq > /dev/null  0.15s user 0.01s system 99% cpu 0.158 total
@mrak

mrak Feb 12, 2014

Contributor
time pacman -Ssq | sed -e 's/$/\tPackage/' >/dev/null
pacman -Ssq  0.15s user 0.01s system 99% cpu 0.158 total
sed -e 's/$/\tPackage/' > /dev/null  0.01s user 0.00s system 4% cpu 0.158 total
time pacman -Ssq >/dev/null                          
pacman -Ssq > /dev/null  0.15s user 0.00s system 99% cpu 0.154 total
time pacman -Slq | sed -e 's/$/\tPackage/' >/dev/null 
pacman -Slq  0.16s user 0.00s system 92% cpu 0.172 total
sed -e 's/$/\tPackage/' > /dev/null  0.03s user 0.00s system 17% cpu 0.172 total
time pacman -Slq >/dev/null                          
pacman -Slq > /dev/null  0.15s user 0.01s system 99% cpu 0.158 total

This comment has been minimized.

@zanchey

zanchey Feb 13, 2014

Member

OK, so there's not a huge amount of difference which is not really a surprise. I think it might be worth caching at that speed as is done for rpm. Any other opinions?

@zanchey

zanchey Feb 13, 2014

Member

OK, so there's not a huge amount of difference which is not really a surprise. I think it might be worth caching at that speed as is done for rpm. Any other opinions?

This comment has been minimized.

@mrak

mrak Feb 14, 2014

Contributor

I noticed that yum and rpm have different cache invalidation times. What was the reasoning? If we cache pacman, what should the timeframe be?

In general I don't think packages are added or removed from the repositories that often. Updated, yes, but that shouldn't affect this function :)

@mrak

mrak Feb 14, 2014

Contributor

I noticed that yum and rpm have different cache invalidation times. What was the reasoning? If we cache pacman, what should the timeframe be?

In general I don't think packages are added or removed from the repositories that often. Updated, yes, but that shouldn't affect this function :)

This comment has been minimized.

@zanchey

zanchey Feb 14, 2014

Member

I don't think there's a huge amount of science behind it, but the rpm command runs in 900 msec and the yum command takes 18-20 seconds on my hardware.

pacman is on the same order of magnitude as rpm, so you could cache for 5 minutes. The cache update runs in the background anyway, so there's no direct hit from it.

@zanchey

zanchey Feb 14, 2014

Member

I don't think there's a huge amount of science behind it, but the rpm command runs in 900 msec and the yum command takes 18-20 seconds on my hardware.

pacman is on the same order of magnitude as rpm, so you could cache for 5 minutes. The cache update runs in the background anyway, so there's no direct hit from it.

@mrak

This comment has been minimized.

Show comment
Hide comment
@mrak

mrak Feb 15, 2014

Contributor

I moved the check for pacman to be before rpm, since it's not uncommon on Arch systems to have rpm installed for handling AUR packages originally packaged for .rpm based distros

Contributor

mrak commented Feb 15, 2014

I moved the check for pacman to be before rpm, since it's not uncommon on Arch systems to have rpm installed for handling AUR packages originally packaged for .rpm based distros

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Feb 16, 2014

Member

Looks good. Merged with rebase here:

To git@github.com:fish-shell/fish-shell.git
de8bae3..64ca6c0 master -> master

Thanks!

Member

zanchey commented Feb 16, 2014

Looks good. Merged with rebase here:

To git@github.com:fish-shell/fish-shell.git
de8bae3..64ca6c0 master -> master

Thanks!

@zanchey zanchey closed this Feb 16, 2014

@zanchey zanchey added the enhancement label Feb 16, 2014

@zanchey zanchey added this to the next-minor milestone Feb 16, 2014

@zanchey zanchey self-assigned this Feb 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment