obihann / archey-osx

An archey script clone for OS X
http://obihann.github.io/archey-osx/
GNU General Public License v2.0
341 stars 124 forks source link

🔋❓ #38

Closed jhersh closed 8 years ago

jhersh commented 8 years ago

archey 1.5.2 on my Mac Pro, which sadly lacks a battery, displays Battery: ? as part of its output.

I think I preferred archey 1.5.1's take, which omitted the battery line entirely on devices that don't have a battery. I am curious if there's any particular reason for this change? :)

obihann commented 8 years ago

@jhersh Thanks for pointing this out. We changed how we poll the battery to speed the startup time, and since I personally don't have a mac without a battery this slipped by me. I'll try to get a fix out for you this week.

Thanks!

childnode commented 8 years ago

yes, confirm, my pr #34 did this, you can easily test this by "mocking" output: https://github.com/childnode/archey-osx/commit/11c680abd33823716063ef4e40531b53968b13ec#diff-eb0567f739a3de3220b8e9fa66e91e97R79

echo "fooBar" | awk '$1~/Capacity/{c[$1]=$3} END{OFMT="%.2f%%"; max=c["\"MaxCapacity\""]; print (max>0? 100*c["\"CurrentCapacity\""]/max: "?")}' will result in ? as mentioned by @jhersh

that's the result blind-copy other guys code from forums without really reading it ;) print (max>0? 100*c["\"CurrentCapacity\""]/max: "?") better if (max>0) { print 100*c["\"CurrentCapacity\""]/max;}

that's where the ? comes from.

if you @obihann refer to this bug on https://github.com/obihann/archey-osx/commit/78c3df37c43c2007c31d0b5d8a7a1f4cec50258c => nope, you will not fix anything with this ;p why do you think formatter change %.2f%% to %.2f% is causing this or will change anything? you'll just get the next bug,

fixed in my develop

obihann commented 8 years ago

@childnode if you check now I am determining when to show / hide the battery display, we check if the value is empty, but your code would display a % sign with no value when no battery was present. By removing the % until we print then we can properly check if the value was empty and not display it... That being said your pull request looks good as well, so if you can simply resolve the merge conflicts I don't see why we can't get @jhersh to test it out then mark it resolved and release it.

childnode commented 8 years ago

hi @obihann

if you check now what? => https://github.com/obihann/archey-osx/commit/78c3df37c43c2007c31d0b5d8a7a1f4cec50258c

and no, the code will not display % sign if no value was parsed when no battery was present. just tested in an VirtualMachine ... The OFMT alias numeric output formatter just formats what is printed, if nothing is printed if no MaxCapacity is found in ioreg, awk must not print "?". That's it!

the "merge conflict" resulted in disregarding your commit https://github.com/obihann/archey-osx/commit/78c3df37c43c2007c31d0b5d8a7a1f4cec50258c.
ommit your change, that's it

HINT: for next release, you should check your differences between master and develop, e.g. README.md which is outdated in develop branch. perhaps you might check the git-flow workflow. it's easy but solves several common workflow problems

jhersh commented 8 years ago

With https://github.com/obihann/archey-osx/commit/78c3df37c43c2007c31d0b5d8a7a1f4cec50258c the output is Battery: ?%

obihann commented 8 years ago

@childnode @jhersh Sorry for the delay, jumping on this now. I've merged your PR into develop and should have a 1.6 release later this week with it.

CamJN commented 8 years ago

Any news on that 1.6 release? I'm a no-battery user as well (actually a laptop with the battery removed) and would appreciate this improvement.

obihann commented 8 years ago

I believe we good, latest code via master should contain the fixes.

obihann commented 8 years ago

This is now in 1.6.0-pr https://github.com/obihann/archey-osx/releases/tag/1.6.0-pr