search

oh-my-fish / plugin-weather

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

Bugfixes math // Update GeoIP #29

Closed miili closed 5 years ago

miili commented 5 years ago

This PR incorporates the bugfixes introduced by https://github.com/oh-my-fish/plugin-weather/pull/28 and https://github.com/oh-my-fish/plugin-weather/pull/24.

ericqweinstein commented 5 years ago

Confirming this works locally (at least for me on macOS Mojave, 10.14.3):

Weather for Santa Monica, United States

Temperature: 13.88 °C (56.984 °F)
   Humidity: 67%
 Cloudiness: broken clouds
   Pressure: 1019 hpa
       Wind: from S (210°) at 2.6 m/s (9.4 km/h)

5-day forecast
  Temperature: █▇▆▅▄▄▃▆▆▆▄▂▁▁▁▆▇▇▄▂▂▂▂▅▆▆▄▂▂▁▁▆██▅▄▂▁▁█
               07/03  08/03  09/03  10/03  11/03  12/03

Precipitation: ▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅▅
               07/03  08/03  09/03  10/03  11/03  12/03
abhigenie92 commented 5 years ago

Can this be merged into the master, please. :)

ahan98 commented 5 years ago

Works perfectly after overwriting the original master files!

Here's the output I got BEFORE this fix:

Weather for ,

math: Error: Expression is bogus
'null * 3.6'
    ^
math: Error: Missing closing parenthesis
'((null % 360) / 45) + 1'
      ^
math: Error: Expression is bogus
'null - 273.15'
    ^
math: Error: Expression is bogus
' * 1.8 + 32'
  ^
Temperature:  °C ( °F)
   Humidity: null%
 Cloudiness: null
   Pressure: null hpa
       Wind: from  (null°) at null m/s ( km/h)

5-day forecast
jq: error (at <stdin>:1): Cannot iterate over null (null)
  Temperature:
  Temperature:     USAGE:
  Temperature:       spark [-h|--help] VALUE,...
  Temperature:
  Temperature:     EXAMPLES:
  Temperature:       spark 1 5 22 13 53
  Temperature:       ▁▁▃▂█
  Temperature:       spark 0,30,55,80,33,150
  Temperature:       ▁▂▃▄▂█
  Temperature:       echo 9 13 5 17 1 | spark
  Temperature:       ▄▆▂█▁

Precipitation:
Precipitation:     USAGE:
Precipitation:       spark [-h|--help] VALUE,...
Precipitation:
Precipitation:     EXAMPLES:
Precipitation:       spark 1 5 22 13 53
Precipitation:       ▁▁▃▂█
Precipitation:       spark 0,30,55,80,33,150
Precipitation:       ▁▂▃▄▂█
Precipitation:       echo 9 13 5 17 1 | spark
Precipitation:       ▄▆▂█▁

And now after:

Weather for XXX, United States

Temperature: 9.81 °C (49.658 °F)
   Humidity: 100%
 Cloudiness: light rain
   Pressure: 1020 hpa
       Wind: from SW (240°) at 3.1 m/s (11.2 km/h)

5-day forecast
  Temperature: ▄▃▃▂▁▁▁▁▃▅▅▃▃▂▂▃▄▅▅▄▃▂▁▁▅▆▇▄▂▂▁▂▇█▇▄▂▂▁▂
               21/03  22/03  23/03  24/03  25/03  26/03

Precipitation: ▁▆█▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁
               21/03  22/03  23/03  24/03  25/03  26/03

(censored city name 😉 )

Please merge into master, and ty @miili !!

hemedani commented 5 years ago

please merge into the master branch

akrz commented 5 years ago

Can someone merge this?

ericqweinstein commented 5 years ago

@scorphus Is this okay to merge? Please let us know if you need any additional info or help! 🙏

jenhausu commented 5 years ago

@scorphus please merge this PR, this plugin can't work without this PR 🙏

scorphus commented 5 years ago

Sorry, closed by accident. Will review and request quite a few changes.

ericqweinstein commented 5 years ago

@miili Do you have time/bandwidth to make the changes @scorphus requested, or would you like me to pick this one up?

miili commented 5 years ago

@miili Do you have time/bandwidth to make the changes @scorphus requested, or would you like me to pick this one up?

Hey @ericqweinstein, please go ahead! Some of the requested changes will break the plugin on my system, please mind my comments.

scorphus commented 5 years ago

As wisely raised by @derekstavis in another question, we need to make sure all changes here are backward compatible with Fish 2.x, so the plugin still works for users or on distros that still didn't upgrade.

rousseldenis commented 5 years ago

Hi all, what's the status of this ?

ericqweinstein commented 5 years ago

@scorphus Since I would have had to be added as a collaborator on @miili's fork, I created a new one (#32)—please let me know if you'd prefer I take a different approach. Happy to make any changes you need.

scorphus commented 5 years ago

If you don't mind, I'd rather squash and merge #29, for authorship reasons. Also, #32 does not add anything on top of that.

Then I'll fix a few issues with it and create another pull request for review.

scorphus commented 5 years ago

Also, let's not forget this is partially a dupe of #28. So I'll rebase these changes on top of those.

scorphus commented 5 years ago

Please review and comment.

ericqweinstein commented 5 years ago

🎉