Skip to content
This repository has been archived by the owner on Feb 14, 2020. It is now read-only.

🔋❓ #38

Closed
jhersh opened this issue Nov 30, 2015 · 9 comments
Closed

🔋❓ #38

jhersh opened this issue Nov 30, 2015 · 9 comments

Comments

@jhersh
Copy link

jhersh commented Nov 30, 2015

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
Copy link
Owner

obihann commented Dec 1, 2015

@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 added a commit to childnodeContribArch/archey-osx that referenced this issue Dec 2, 2015
…conditional else case if battery max value is not found resulted in ? instead of NULL
@childnode
Copy link
Contributor

yes, confirm, my pr #34 did this, you can easily test this by "mocking" output:
childnodeContribArch@11c680a#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 78c3df3
=> 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
Copy link
Owner

obihann commented Dec 2, 2015

@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
Copy link
Contributor

hi @obihann

if you check now
what? => 78c3df3

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 78c3df3.
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
Copy link
Author

jhersh commented Dec 5, 2015

With 78c3df3 the output is Battery: ?%

@obihann obihann added the bug label Dec 21, 2015
@obihann obihann self-assigned this Dec 21, 2015
@obihann obihann modified the milestone: 1.5.2 Dec 21, 2015
@obihann obihann added the 1.5.2 label Dec 21, 2015
@obihann obihann modified the milestone: 1.6 Dec 21, 2015
@obihann obihann assigned obihann and unassigned obihann Dec 21, 2015
@obihann
Copy link
Owner

obihann commented Dec 21, 2015

@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
Copy link

CamJN commented Jan 12, 2016

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 obihann mentioned this issue Feb 25, 2016
@obihann
Copy link
Owner

obihann commented Feb 25, 2016

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

@obihann obihann closed this as completed Feb 25, 2016
@obihann
Copy link
Owner

obihann commented Feb 25, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants