* bug#18157: 24.4.50; battery-pmset fails to report critical battery state @ 2014-07-31 13:16 Sebastian Wiesner 2016-02-16 14:56 ` Marcin Borkowski 0 siblings, 1 reply; 8+ messages in thread From: Sebastian Wiesner @ 2014-07-31 13:16 UTC (permalink / raw) To: 18157 `battery-pmset' fails to correctly report critical battery state: Even if the battery charge level is below `battery-load-critical', it reports "low" battery state only. The culprit is the `cond' expression in the body of `battery-pmset' which compares the reported `load-percentage' against `battery-load-low' first, and then against `battery-load-critical'. Since the latter is typically lower, it will never be reached, because `cond' already returns after the former succeeded. To fix this issue, `batter-pmset' needs to check `battery-load-critical' *first*, and then `battery-load-low'. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2014-07-31 13:16 bug#18157: 24.4.50; battery-pmset fails to report critical battery state Sebastian Wiesner @ 2016-02-16 14:56 ` Marcin Borkowski 2016-02-16 18:41 ` Sebastian Wiesner 2016-02-20 7:35 ` Lars Ingebrigtsen 0 siblings, 2 replies; 8+ messages in thread From: Marcin Borkowski @ 2016-02-16 14:56 UTC (permalink / raw) To: Sebastian Wiesner; +Cc: mbork, 18157 On 2014-07-31, at 16:16, Sebastian Wiesner <swiesner@lunaryorn.com> wrote: > `battery-pmset' fails to correctly report critical battery state: Even > if the battery charge level is below `battery-load-critical', it reports > "low" battery state only. > > The culprit is the `cond' expression in the body of `battery-pmset' > which compares the reported `load-percentage' against `battery-load-low' > first, and then against `battery-load-critical'. > > Since the latter is typically lower, it will never be reached, because > `cond' already returns after the former succeeded. > > To fix this issue, `batter-pmset' needs to check `battery-load-critical' > *first*, and then `battery-load-low'. Does this patch help? --8<---------------cut here---------------start------------->8--- diff -u --label /usr/local/share/emacs/25.1.50/lisp/battery.el.gz --label \#\<buffer\ battery.el.gz\> /tmp/jka-com1889YvM /tmp/buffer-content-1889yDZ --- /usr/local/share/emacs/25.1.50/lisp/battery.el.gz +++ #<buffer battery.el.gz> @@ -628,12 +628,12 @@ (cond ((looking-at "; charging") (setq battery-status "charging" battery-status-symbol "+")) - ((< (string-to-number load-percentage) battery-load-low) - (setq battery-status "low" - battery-status-symbol "-")) ((< (string-to-number load-percentage) battery-load-critical) (setq battery-status "critical" battery-status-symbol "!")) + ((< (string-to-number load-percentage) battery-load-low) + (setq battery-status "low" + battery-status-symbol "-")) (t (setq battery-status "high" battery-status-symbol ""))) Diff finished. Tue Feb 16 15:56:03 2016 --8<---------------cut here---------------end--------------->8--- Best, -- mb ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2016-02-16 14:56 ` Marcin Borkowski @ 2016-02-16 18:41 ` Sebastian Wiesner 2016-02-16 19:04 ` Eli Zaretskii 2016-02-16 23:43 ` Marcin Borkowski 2016-02-20 7:35 ` Lars Ingebrigtsen 1 sibling, 2 replies; 8+ messages in thread From: Sebastian Wiesner @ 2016-02-16 18:41 UTC (permalink / raw) To: Marcin Borkowski; +Cc: 18157 > Am 16.02.2016 um 15:56 schrieb Marcin Borkowski <mbork@mbork.pl>: > > On 2014-07-31, at 16:16, Sebastian Wiesner <swiesner@lunaryorn.com> wrote: > >> `battery-pmset' fails to correctly report critical battery state: Even >> if the battery charge level is below `battery-load-critical', it reports >> "low" battery state only. >> >> The culprit is the `cond' expression in the body of `battery-pmset' >> which compares the reported `load-percentage' against `battery-load-low' >> first, and then against `battery-load-critical'. >> >> Since the latter is typically lower, it will never be reached, because >> `cond' already returns after the former succeeded. >> >> To fix this issue, `batter-pmset' needs to check `battery-load-critical' >> *first*, and then `battery-load-low'. > > Does this patch help? I'm sorry, are you asking me? It would be bold to believe that I still cared for this issue now, almost *two years* after I reported it. At a cursory look the fix surely looks correct, but I hope you understand that Emacs taking two years for what seems to be a trivial issue to fix does not particularly contribute to my enthusiasm, nor does it motivate me to test this patch. > --8<---------------cut here---------------start------------->8--- > diff -u --label /usr/local/share/emacs/25.1.50/lisp/battery.el.gz --label \#\<buffer\ battery.el.gz\> /tmp/jka-com1889YvM /tmp/buffer-content-1889yDZ > --- /usr/local/share/emacs/25.1.50/lisp/battery.el.gz > +++ #<buffer battery.el.gz> > @@ -628,12 +628,12 @@ > (cond ((looking-at "; charging") > (setq battery-status "charging" > battery-status-symbol "+")) > - ((< (string-to-number load-percentage) battery-load-low) > - (setq battery-status "low" > - battery-status-symbol "-")) > ((< (string-to-number load-percentage) battery-load-critical) > (setq battery-status "critical" > battery-status-symbol "!")) > + ((< (string-to-number load-percentage) battery-load-low) > + (setq battery-status "low" > + battery-status-symbol "-")) > (t > (setq battery-status "high" > battery-status-symbol ""))) > > Diff finished. Tue Feb 16 15:56:03 2016 > --8<---------------cut here---------------end--------------->8--- > > Best, > > -- > mb ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2016-02-16 18:41 ` Sebastian Wiesner @ 2016-02-16 19:04 ` Eli Zaretskii 2016-02-20 5:51 ` John Wiegley 2016-02-16 23:43 ` Marcin Borkowski 1 sibling, 1 reply; 8+ messages in thread From: Eli Zaretskii @ 2016-02-16 19:04 UTC (permalink / raw) To: Sebastian Wiesner; +Cc: mbork, 18157 > From: Sebastian Wiesner <swiesner@lunaryorn.com> > Date: Tue, 16 Feb 2016 19:41:25 +0100 > Cc: 18157@debbugs.gnu.org > > I hope you understand that Emacs taking two years for what seems to be a trivial issue to fix does not particularly contribute to my enthusiasm, nor does it motivate me to test this patch. FWIW, I'm very grateful to Marcin and a few people like him for investing their time into working on this and other bug reports, and hope that more contributors like them will come on board and help in this effort. Thanks a lot, guys! ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2016-02-16 19:04 ` Eli Zaretskii @ 2016-02-20 5:51 ` John Wiegley 0 siblings, 0 replies; 8+ messages in thread From: John Wiegley @ 2016-02-20 5:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: mbork, Sebastian Wiesner, 18157 [-- Attachment #1: Type: text/plain, Size: 531 bytes --] >>>>> Eli Zaretskii <eliz@gnu.org> writes: > FWIW, I'm very grateful to Marcin and a few people like him for investing > their time into working on this and other bug reports, and hope that more > contributors like them will come on board and help in this effort. > > Thanks a lot, guys! I would like to second that thanks. We move at the speed of our volunteers. -- John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 625 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2016-02-16 18:41 ` Sebastian Wiesner 2016-02-16 19:04 ` Eli Zaretskii @ 2016-02-16 23:43 ` Marcin Borkowski 2016-02-20 7:36 ` Lars Ingebrigtsen 1 sibling, 1 reply; 8+ messages in thread From: Marcin Borkowski @ 2016-02-16 23:43 UTC (permalink / raw) To: Sebastian Wiesner; +Cc: 18157 On 2016-02-16, at 19:41, Sebastian Wiesner <swiesner@lunaryorn.com> wrote: >> Does this patch help? > > I'm sorry, are you asking me? It would be bold to believe that I still cared for this issue now, almost *two years* after I reported it. > > At a cursory look the fix surely looks correct, but I hope you understand that Emacs taking two years for what seems to be a trivial issue to fix does not particularly contribute to my enthusiasm, nor does it motivate me to test this patch. I am also sorry if I sounded rude. The question was directed to people reading the bug-gnu-emacs mailing list as well as to you. I am also sorry that this was not fixed earlier, but I am bold enough to believe that it is better to try to investigate it now than to leave it forever. I am also sorry that I didn't test this myself. However, I do not own a Mac, so I could only transcribe your report into a (trivial, admittedly) patch for others to test. I do not consider my input here too valuable (everyone could write this patch, even without any deeper knowledge about Elisp, thanks to your detailed report and suggestion as to were the correct fix was - thank you for that!), but I hoped that what I may have /really/ fixed was the problem of your report not gaining enough attention when it was sent. Also, even if you don't care for this issue anymore, there may be other people who do. And you were the author of the report, so it was a natural thing to send this message to you. (In fact, this is what the debbugs tool did, as it does by default when responding to bug reports.) Also, if I should have behaved differently, please tell me what I could have done better (apart from the not-so-optimal wording of my message). Best, -- Marcin Borkowski http://octd.wmi.amu.edu.pl/en/Marcin_Borkowski Faculty of Mathematics and Computer Science Adam Mickiewicz University ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2016-02-16 23:43 ` Marcin Borkowski @ 2016-02-20 7:36 ` Lars Ingebrigtsen 0 siblings, 0 replies; 8+ messages in thread From: Lars Ingebrigtsen @ 2016-02-20 7:36 UTC (permalink / raw) To: Marcin Borkowski; +Cc: Sebastian Wiesner, 18157 Marcin Borkowski <mbork@mbork.pl> writes: > Also, if I should have behaved differently, please tell me what I could > have done better (apart from the not-so-optimal wording of my message). No, you've done exactly what we hope people would do. Thank you for investigating the problem and providing a patch. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#18157: 24.4.50; battery-pmset fails to report critical battery state 2016-02-16 14:56 ` Marcin Borkowski 2016-02-16 18:41 ` Sebastian Wiesner @ 2016-02-20 7:35 ` Lars Ingebrigtsen 1 sibling, 0 replies; 8+ messages in thread From: Lars Ingebrigtsen @ 2016-02-20 7:35 UTC (permalink / raw) To: Marcin Borkowski; +Cc: 18157 Marcin Borkowski <mbork@mbork.pl> writes: > Does this patch help? > > diff -u --label /usr/local/share/emacs/25.1.50/lisp/battery.el.gz --label \#\<buffer\ battery.el.gz\> /tmp/jka-com1889YvM /tmp/buffer-content-1889yDZ > --- /usr/local/share/emacs/25.1.50/lisp/battery.el.gz > +++ #<buffer battery.el.gz> > @@ -628,12 +628,12 @@ > (cond ((looking-at "; charging") > (setq battery-status "charging" > battery-status-symbol "+")) > - ((< (string-to-number load-percentage) battery-load-low) > - (setq battery-status "low" > - battery-status-symbol "-")) > ((< (string-to-number load-percentage) battery-load-critical) > (setq battery-status "critical" > battery-status-symbol "!")) > + ((< (string-to-number load-percentage) battery-load-low) > + (setq battery-status "low" > + battery-status-symbol "-")) > (t > (setq battery-status "high" > battery-status-symbol ""))) Looks logical to me. I've applied it to emacs-25. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-20 7:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-31 13:16 bug#18157: 24.4.50; battery-pmset fails to report critical battery state Sebastian Wiesner 2016-02-16 14:56 ` Marcin Borkowski 2016-02-16 18:41 ` Sebastian Wiesner 2016-02-16 19:04 ` Eli Zaretskii 2016-02-20 5:51 ` John Wiegley 2016-02-16 23:43 ` Marcin Borkowski 2016-02-20 7:36 ` Lars Ingebrigtsen 2016-02-20 7:35 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).