oh-my-fish / plugin-weather

A simple, location-aware weather command for Fish
MIT License
31 stars 15 forks source link

jq is not installed by default on all systems #10

Closed mpiffault closed 8 years ago

mpiffault commented 8 years ago

Hi,

I use Archlinux and had an error when I first launched weather because jq command was not installed. Worked great after installation from community packages. Maybe you could add a check before execution ?

Thanks,

sagebind commented 8 years ago

Thanks for the suggestion. In reality, jq isn't pre-installed on any systems that I know of, but it is by far the fastest and best tool for the job.

Would it make sense to show a warning when starting the shell, or when trying to run the command?

mpiffault commented 8 years ago

So looking into the code, it seams you already do the check just after init, but I didn't saw the message.

I think a warning when trying to run the command would be better, because the stack trace when crashing is not nice :

 ~/p/b/n/e/spring-batch-example   master  weather            ven. 11 déc. 2015 19:10:39 CET
fish: Unknown command 'jq'
~/.local/share/omf/pkg/weather/functions/weather.location.fish (line 14):   echo $geoip_data | jq '.location.latitude'
                                                                                               ^
in function “weather.location”
    called on line 2 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in command substitution
    called on line 0 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in function “weather”
    called on standard input

fish: Unknown command 'jq'
~/.local/share/omf/pkg/weather/functions/weather.location.fish (line 15):   echo $geoip_data | jq '.location.longitude'
                                                                                               ^
in function “weather.location”
    called on line 2 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in command substitution
    called on line 0 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in function “weather”
    called on standard input

fish: Unknown command 'jq'
~/.local/share/omf/pkg/weather/functions/weather.location.fish (line 16):   echo $geoip_data | jq -r '.city?'
                                                                                               ^
in function “weather.location”
    called on line 2 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in command substitution
    called on line 0 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in function “weather”
    called on standard input

fish: Unknown command 'jq'
~/.local/share/omf/pkg/weather/functions/weather.location.fish (line 17):   echo $geoip_data | jq -r '.country.name'
                                                                                               ^
in function “weather.location”
    called on line 2 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in command substitution
    called on line 0 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in function “weather”
    called on standard input

Array index out of bounds
~/.local/share/omf/pkg/weather/functions/weather.fish (line 1): weather.fetch "http://api.openweathermap.org/data/2.5/weather" lat=$location[1] lon=$location[2] APPID=$weather_api_key
                                                                                                                                                              ^
in command substitution
    called on line 0 of file ~/.local/share/omf/pkg/weather/functions/weather.fish

in function “weather”
    called on standard input

Unable to fetch weather data; please try again later.
sagebind commented 8 years ago

Yeah, I agree with you. I'll get that fixed ASAP.

sagebind commented 8 years ago

I think you will find f9c4d622 to your liking, no?

omf update
mpiffault commented 8 years ago

Yes it's just what's needed, thanks !