Have ACPI use only the first battery value

matchai / spacefish

🚀🐟 The fish shell prompt for astronauts

https://spacefish.matchai.dev

MIT License

963 stars 79 forks source link

Have ACPI use only the first battery value #126

Closed matchai closed 5 years ago

matchai commented 5 years ago

Description

Ported the solution used in the following PR: https://github.com/denysdovhan/spaceship-prompt/pull/583

Motivation and Context

Closes #124

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots (if appropriate):

How Has This Been Tested?

[ ] I have tested using MacOS
[ ] I have tested using Linux

This feature is untested as I have no computer using acpi. I would appreciate it if someone could validate this fix.

Checklist:

[x] I have checked that no other PR duplicates mine
[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have updated the tests accordingly.

Snuggle commented 5 years ago

As for testing, I'll be happy to test uhh... Tomorrow if I have free time and can remember!

matchai commented 5 years ago

Thank you @Snuggle! Hope you had some enjoyable time off. 😎🌴 I've also been taking it easy the last couple weeks.

Sounds perfect! No pressure if you don't get around to it right away. 😄

matchai commented 5 years ago

@Snuggle Just a friendly reminder that this PR needs testing if you've got some free time. 😄 (Happy new year! 🎉)

Snuggle commented 5 years ago

After testing, my prompt uses my wireless mouse's battery and reports 0%. No errors though! :clinking_glasses:

screenshot from 2019-01-09 09-25-27

Ubuntu 18.10 on a generic Samsung laptop and a Logitech MX Master (1st gen.) wireless mouse.

Snuggle commented 5 years ago

I can't remember where the conversation was, either on this repo or Spaceship, but there needs to be some logic in place to ignore a 0% battery level. Perhaps to return the first non-zero battery level.

Snuggle commented 5 years ago

This pull request does fix the following bug, though and is better than master. I'll try and join in @salmanulfarzy's discussion on how to handle multiple batteries, should be interesting. :slightly_smiling_face:

screenshot from 2019-01-09 09-33-29

matchai commented 5 years ago

Yep! This is by no means a perfect solution. Just a better one for the time being. 😃 Thanks for taking the time to manually test it! 🍻

salmanulfarzy commented 5 years ago

Did you accidentally revert the original changes (2e2aa9980a44a2074d412bf556843b1867b11738) with this merge commit (2c7bd67d78c94b61a50f61d19dd0d4aea1116baa) or was it intentional ? Seems like the only change now is comment removal :man_shrugging:

matchai commented 5 years ago

Oh whoops. That was definitely a slip-up! 😱 Solid catch @salmanulfarzy!

matchai commented 5 years ago

:tada: This PR is included in version 2.0.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: