franciscolourenco / done

A fish-shell package to automatically receive notifications when long processes finish.
MIT License
770 stars 70 forks source link

test: Missing argument at index 2 #19

Closed balupton closed 7 years ago

balupton commented 7 years ago
balupton@balbook ~> fisher done
Installing 1 plugin/s
OK Fetch done github.com/fisherman/done
Installing 1 dependency
OK Fetch humanize_duration github.com/fisherman/humanize_duration
Done in 5s 980ms
balupton@balbook ~> ping -t 5 google.com
PING google.com (216.58.200.110): 56 data bytes
64 bytes from 216.58.200.110: icmp_seq=0 ttl=55 time=55.864 ms
64 bytes from 216.58.200.110: icmp_seq=1 ttl=55 time=58.456 ms
64 bytes from 216.58.200.110: icmp_seq=2 ttl=55 time=57.946 ms
64 bytes from 216.58.200.110: icmp_seq=3 ttl=55 time=58.496 ms
64 bytes from 216.58.200.110: icmp_seq=4 ttl=55 time=56.049 ms

--- google.com ping statistics ---
5 packets transmitted, 5 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 55.864/57.362/58.496/1.165 ms
balupton@balbook ~> ping -t 15 google.com
PING google.com (216.58.200.110): 56 data bytes
64 bytes from 216.58.200.110: icmp_seq=0 ttl=55 time=55.806 ms
64 bytes from 216.58.200.110: icmp_seq=1 ttl=55 time=58.316 ms
64 bytes from 216.58.200.110: icmp_seq=2 ttl=55 time=55.934 ms
64 bytes from 216.58.200.110: icmp_seq=3 ttl=55 time=55.723 ms
64 bytes from 216.58.200.110: icmp_seq=4 ttl=55 time=58.599 ms
64 bytes from 216.58.200.110: icmp_seq=5 ttl=55 time=58.707 ms
64 bytes from 216.58.200.110: icmp_seq=6 ttl=55 time=55.607 ms
64 bytes from 216.58.200.110: icmp_seq=7 ttl=55 time=58.368 ms
64 bytes from 216.58.200.110: icmp_seq=8 ttl=55 time=58.910 ms
64 bytes from 216.58.200.110: icmp_seq=9 ttl=55 time=58.689 ms
64 bytes from 216.58.200.110: icmp_seq=10 ttl=55 time=58.462 ms
64 bytes from 216.58.200.110: icmp_seq=11 ttl=55 time=58.585 ms
64 bytes from 216.58.200.110: icmp_seq=12 ttl=55 time=55.883 ms
64 bytes from 216.58.200.110: icmp_seq=13 ttl=55 time=58.740 ms
64 bytes from 216.58.200.110: icmp_seq=14 ttl=55 time=59.470 ms

--- google.com ping statistics ---
15 packets transmitted, 15 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 55.607/57.720/59.470/1.389 ms
test: Missing argument at index 2
franciscolourenco commented 7 years ago

Thanks @balupton! This should only happen after install, so in new sessions it will work, but I'll look into fixing the problem.

@jbucaran $__done_initial_window_id is always empty in the session where done is installed. I assume it is due to the way fisherman is sourcing the files on install?

This needs variable needs to be local to the session.

jorgebucaran commented 7 years ago

I guess so.

franciscolourenco commented 7 years ago

@jbucaran any idea for a fix?

jorgebucaran commented 7 years ago

Sorry, I don't understand the issue. What's the problem?

franciscolourenco commented 7 years ago

@jbucaran $__done_initial_window_id is not (being set/available) in the session where fisherman/done is installed.

  1. Install fisherman/done for the first time
  2. Execute sleep 11
  3. You get test: Missing argument at index 2 coming from this line
  4. Open a new fish shell
  5. type sleep 11
  6. It works fine
jorgebucaran commented 7 years ago

@aristidesfl Make sure to declare __done_initial_window_id as a global variable.

set -g __done_initial_window_id
franciscolourenco commented 7 years ago

@jbucaran won't that break the plugin if two different windows are being used at the same time? Never mind, I confused global with universal variables. 😅

franciscolourenco commented 7 years ago

Fixed in 1.2.2

@jbucaran thanks! fisherman doesn't seem to always install the latest version without update. Is there some caching going on?

jorgebucaran commented 7 years ago

@aristidesfl Ah, yes, stuff in the cache takes precedence. Perhaps we could force to download fresh and use the cache only if that (download) fails.

franciscolourenco commented 7 years ago

@jbucaran I think that would better satisfy expectations, talking from the point of view of someone used to homebrew.

balupton commented 7 years ago

@aristidesfl Ah, yes, stuff in the cache takes precedence. Perhaps we could force to download fresh and use the cache only if that (download) fails.

+1 on that, even knowing this behaviour it is still confusing. For me, I had run fisher rm done, then fisher done - and it installed from cache again (there is also no message to say that it installed from cache). So it seems one has to install then update, which isn't intuitive.