* Package critique: modeline for air quality information @ 2023-09-01 1:58 John Haman 2023-09-01 9:38 ` Eshel Yaron ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: John Haman @ 2023-09-01 1:58 UTC (permalink / raw) To: help-gnu-emacs I wrote a package that adds local air quality statistics to the mode-line. Here it is: https://github.com/jthaman/air-quality/blob/main/air-quality.el If you are so inclined, I'd like some thoughts on the code. It's short, but I'm trying to get better at Emacs Lisp (at least this week, while I'm on vacation...) -- Dr. John Haman Maryland, USA ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-01 1:58 Package critique: modeline for air quality information John Haman @ 2023-09-01 9:38 ` Eshel Yaron 2023-09-01 10:49 ` John Haman 2023-09-01 15:12 ` tpeplt 2023-09-02 21:24 ` Emanuel Berg 2 siblings, 1 reply; 9+ messages in thread From: Eshel Yaron @ 2023-09-01 9:38 UTC (permalink / raw) To: John Haman; +Cc: help-gnu-emacs Hello John, John Haman <mail@johnhaman.org> writes: > I wrote a package that adds local air quality statistics to the > mode-line. Cool! > > https://github.com/jthaman/air-quality/blob/main/air-quality.el > > If you are so inclined, I'd like some thoughts on the code. It's > short, but I'm trying to get better at Emacs Lisp (at least this week, > while I'm on vacation...) > My suggestion would be to set this up as a global minor mode. Also, you may want to define your "public variables" as user options, so they can be customized with `M-x customize` and friends. Something along the lines of: diff --git a/air-quality.el b/air-quality.el index 1781515..533ce1b 100644 --- a/air-quality.el +++ b/air-quality.el @@ -6,7 +6,7 @@ ;;; air-quality.el --- Air quality modeline indicator and reporting tool -*- le ;; URL: https://github.com/jthaman/air-quality ;; Version: 0.1 ;; Package-Requires: ((emacs "27.1")) -;; Keywords: bling +;; Keywords: convenience ;; This program is free software; you can redistribute it and/or modify ;; it under the terms of the GNU General Public License as published by @@ -21,35 +21,44 @@ ;;; air-quality.el --- Air quality modeline indicator and reporting tool -*- le ;; You should have received a copy of the GNU General Public License ;; along with this program. If not, see <https://www.gnu.org/licenses/>. -;;; Package Imports -(require 'url) -(require 'json) -(require 'cl-lib) - - ;;; Commentary: + ;; Add information about local air quality to the modeline. Air Quality ;; information is downloaded from the Open Weather Map Air Pollution API. - ;;; Code: +;;;; Package Imports + +(require 'url) +(require 'json) +(require 'cl-lib) + +;;;; User options -;;; Public Variables -(defvar air-quality-open-weather-api-key nil - "A string. API key for Open Weather Map.") +(defgroup air-quality nil + "Air quality mode-line indicator." + :group 'mode-line) + +(defcustom air-quality-open-weather-api-key nil + "API key for Open Weather Map." + :type '(choice (const :tag "Unset" nil) + (string :tag "Your API key"))) (defvar air-quality-refresh-interval 60 + ;; XXX - turn into a defcustom "An integer. Number of minutes between refreshes of air quality information.") (defvar air-quality-latitude nil + ;; XXX - likewise "A float. Your latitude.") (defvar air-quality-longitude nil + ;; XXX - likewise "A float. Your longitude.") +;;;; Private Variables -;;; Private Variables (defvar air-quality--timer nil) (defvar air-quality--co nil "Carbon Monoxide level (micrograms per cubic-meter).") @@ -62,30 +71,44 @@ (defvar air-quality--pm10 nil "PM 10 level (micrograms per cubic-meter).") (defvar air-quality--nh3 nil "Ammonia level (micrograms per cubic-meter).") (defvar air-quality--level nil "Overall Air Quality (AQI).") -(defun air-quality--make-forecast-call (key lat lon) - "Create an API request for the future forecast of Air Quality information from Open Weather Map." - (concat "http://api.openweathermap.org/data/2.5/air_pollution/forecast?lat=" - (number-to-string lat) - "&lon=" - (number-to-string lon) - "&appid=" - key)) - -(defun air-quality--make-api-call (key lat lon) - "Create an API request to Open Weather Map." - (concat "http://api.openweathermap.org/data/2.5/air_pollution?lat=" - (number-to-string lat) - "&lon=" - (number-to-string lon) - "&appid=" - key)) - -(defvar air-quality--index-alist '((1 . "Good") - (2 . "Fair") - (3 . "Moderate") - (4 . "Poor") - (5 . "Very Poor"))) - +(defun air-quality--make-api-call (key latitude longitude) + "Return Open Weather Map URL for LATITUDE and LONGITUDE with API key KEY." + (url-parse-make-urlobj + "https" nil nil "api.openweathermap.org" nil + (concat "/data/2.5/air_pollution/?" + (url-build-query-string `(("lat" ,latitude) + ("lon" ,longitude) + ("appid" ,key)))))) + +(defconst air-quality--index-alist '((1 . "Good") + (2 . "Fair") + (3 . "Moderate") + (4 . "Poor") + (5 . "Very Poor"))) + +(defvar air-quality-indicator + '(:eval + (propertize (concat " " (alist-get air-quality--level + air-quality--index-alist)) + 'face 'mode-line-buffer-id + 'help-echo (format "Carbon Monoxide: %d µg/m³ +Nitrogen Oxide: %d µg/m³ +Nitrogen Dioxide: %d µg/m³ +Ozone: %d µg/m³ +Sulfur Dioxide: %d µg/m³ +PM 2.5: %d µg/m³ +PM 10: %d µg/m³ +Ammonia: %d µg/m³" + air-quality--co + air-quality--no + air-quality--no2 + air-quality--o3 + air-quality--so2 + air-quality--pm2_5 + air-quality--pm10 + air-quality--nh3) + 'mouse-face 'mode-line-highlight))) +(put 'air-quality-indicator 'risky-local-variable t) (defun air-quality--get-update () "Query Open Weather for air quality information." @@ -93,7 +116,7 @@ (defun air-quality--get-update () (air-quality--make-api-call air-quality-open-weather-api-key air-quality-latitude air-quality-longitude) - (lambda (events) + (lambda (_events) (goto-char url-http-end-of-headers) (let ((json-object-type 'plist) (json-key-type 'symbol) @@ -108,57 +131,24 @@ (defun air-quality--get-update () (setq air-quality--pm2_5 (plist-get components 'pm2_5)) (setq air-quality--pm10 (plist-get components 'pm10)) (setq air-quality--nh3 (plist-get components 'nh3)) - (setq air-quality--level (cadadr (aref (plist-get result 'list) 0))) - (air-quality--set-indicator) - (run-with-idle-timer 1 nil #'air-quality--append-modeline) - ))))) - -(defun air-quality--set-indicator () - (defvar-local air-quality-indicator - (list (propertize (concat " " (alist-get air-quality--level air-quality--index-alist)) - 'face 'mode-line-buffer-id - 'help-echo (purecopy (format "Carbon Monoxide: %d µg/m³ -Nitrogen Oxide: %d µg/m³ -Nitrogen Dioxide: %d µg/m³ -Ozone: %d µg/m³ -Sulfur Dioxide: %d µg/m³ -PM 2.5: %d µg/m³ -PM 10: %d µg/m³ -Ammonia: %d µg/m³" - air-quality--co - air-quality--no - air-quality--no2 - air-quality--o3 - air-quality--so2 - air-quality--pm2_5 - air-quality--pm10 - air-quality--nh3)) - 'mouse-face 'mode-line-highlight - ))) - (put 'air-quality-indicator 'risky-local-variable t)) - - -(defun air-quality--append-modeline () - (unless (cl-find '(:eval air-quality-indicator) mode-line-format :test 'equal) - (setq-default mode-line-format - (reverse (append '((:eval air-quality-indicator)) (reverse mode-line-format)))))) + (setq air-quality--level (cadadr (aref (plist-get result 'list) 0)))))))) ;;;###autoload -(defun air-quality-setup-modeline () - - ;; This could be better... - ;; if no timer, start one - (if (null air-quality--timer) - (setq air-quality--timer - (run-with-timer 0 (* 60 air-quality-refresh-interval) #'air-quality--get-update)) - ;; otherwise, cancel the timer and start one - (progn - (cancel-timer air-quality--timer) - (setq air-quality--timer - (run-with-timer 0 (* 60 air-quality-refresh-interval) #'air-quality--get-update)) - ))) - +(define-minor-mode air-quality-mode + "Minor mode for displaying air quality information in the mode line." + :group 'air-quality + :global t + (if air-quality-mode + (progn + ;; XXX - check that air-quality-open-weather-api-key, + ;; air-quality-latitude and air-quality-longitude are set + (add-to-list 'mode-line-misc-info 'air-quality-indicator t ) + (setq air-quality--timer + (run-with-timer 0 (* 60 air-quality-refresh-interval) + #'air-quality--get-update))) + (setq mode-line-misc-info (delq 'air-quality-indicator mode-line-misc-info)) + (cancel-timer air-quality--timer)) + (force-mode-line-update)) (provide 'air-quality) - ;;; air-quality.el ends here > -- > Dr. John Haman > Maryland, USA ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-01 9:38 ` Eshel Yaron @ 2023-09-01 10:49 ` John Haman 2023-09-01 11:57 ` Eshel Yaron 0 siblings, 1 reply; 9+ messages in thread From: John Haman @ 2023-09-01 10:49 UTC (permalink / raw) To: Eshel Yaron; +Cc: help-gnu-emacs Thank you for the fast and thorough response! This definitely makes the package better. I applied the patch. I had some 404 issue with the new version of `air-quality--make-api-call', so I reverted back to the old one that just uses a simple string concatenation. Is that OK? Or is there some specific advantage to using `url-parse-make-urlobj' ? https://github.com/jthaman/air-quality You might check to see if I implemented the variable checking in a correct way in the definition of `air-quality-mode'. -John -- Dr. John Haman Maryland, USA ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-01 10:49 ` John Haman @ 2023-09-01 11:57 ` Eshel Yaron 2023-09-01 13:46 ` John Haman 0 siblings, 1 reply; 9+ messages in thread From: Eshel Yaron @ 2023-09-01 11:57 UTC (permalink / raw) To: John Haman; +Cc: help-gnu-emacs [-- Attachment #1: Type: text/plain, Size: 1022 bytes --] John Haman <mail@johnhaman.org> writes: > > I had some 404 issue with the new version of > `air-quality--make-api-call', so I reverted back to the old one that > just uses a simple string concatenation. Is that OK? Sure, > Or is there some specific advantage to using `url-parse-make-urlobj' ? Not that I know of, I just find it a bit cleaner. > > https://github.com/jthaman/air-quality > > You might check to see if I implemented the variable checking in a correct way in the definition of `air-quality-mode'. > Looks good, a possible improvement in that area would be to prompt the user for these settings when they try to activate the mode, instead of silently failing. Also note that for the `defcustom`s, if you set the `:type` to `number` then the default value `nil` isn't of the right type. You can see this mismatch indicated in the Custom buffer, for example. Here are two patches to make these suggestions concrete, feel free to apply them as they are, ignore them completely, or anything in between: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Refine-defcustom-types.patch --] [-- Type: text/x-patch, Size: 1198 bytes --] From 0b6f014db9d275d6ea6f3b1e04d3389018fd1705 Mon Sep 17 00:00:00 2001 From: Eshel Yaron <me@eshelyaron.com> Date: Fri, 1 Sep 2023 13:40:34 +0200 Subject: [PATCH 1/2] Refine 'defcustom' types --- air-quality.el | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/air-quality.el b/air-quality.el index 8f55100..7895b8c 100644 --- a/air-quality.el +++ b/air-quality.el @@ -45,16 +45,18 @@ (defcustom air-quality-open-weather-api-key nil (string :tag "Your API key"))) (defcustom air-quality-refresh-interval 60 - "An integer. Number of minutes between refreshes of air quality information." - :type 'number) + "Number of minutes between refreshes of air quality information." + :type 'integer) (defcustom air-quality-latitude nil - "A float. Your latitude." - :type 'number) + "Your latitude." + :type '(choice (const :tag "Unset" nil) + (integer :tag "Your latitude"))) (defcustom air-quality-longitude nil - "A float. Your longitude." - :type 'number) + "Your longitude." + :type '(choice (const :tag "Unset" nil) + (integer :tag "Your longitude"))) ;;;; Private Variables -- 2.42.0 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: attachment --] [-- Type: text/x-patch, Size: 2190 bytes --] From bc4c729ab26e34515cbc9d90ca182af4a8939a00 Mon Sep 17 00:00:00 2001 From: Eshel Yaron <me@eshelyaron.com> Date: Fri, 1 Sep 2023 13:42:22 +0200 Subject: [PATCH 2/2] Prompt for missing settings --- air-quality.el | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/air-quality.el b/air-quality.el index 7895b8c..c6d2250 100644 --- a/air-quality.el +++ b/air-quality.el @@ -144,14 +144,27 @@ (define-minor-mode air-quality-mode :group 'air-quality :global t (if air-quality-mode - (progn - ;; Check that air-quality-open-weather-api-key, - ;; air-quality-latitude and air-quality-longitude are set - (when (and air-quality-open-weather-api-key air-quality-latitude air-quality-longitude) - (add-to-list 'mode-line-misc-info 'air-quality-indicator t ) - (setq air-quality--timer - (run-with-timer 0 (* 60 air-quality-refresh-interval) - #'air-quality--get-update)))) + (let ((need-save nil)) + (unless air-quality-open-weather-api-key + (setq air-quality-open-weather-api-key + (read-string "Set your Open Weather API key: ") + need-save t) + (customize-mark-to-save 'air-quality-open-weather-api-key)) + (unless air-quality-latitude + (setq air-quality-latitude + (read-number "Set your latitude: ") + need-save t) + (customize-mark-to-save 'air-quality-latitude)) + (unless air-quality-longitude + (setq air-quality-longitude + (read-number "Set your longitude: ") + need-save t) + (customize-mark-to-save 'air-quality-longitude)) + (when need-save (custom-save-all)) + (add-to-list 'mode-line-misc-info 'air-quality-indicator t ) + (setq air-quality--timer + (run-with-timer 0 (* 60 air-quality-refresh-interval) + #'air-quality--get-update))) (setq mode-line-misc-info (delq 'air-quality-indicator mode-line-misc-info)) (cancel-timer air-quality--timer)) (force-mode-line-update)) -- 2.42.0 [-- Attachment #4: Type: text/plain, Size: 63 bytes --] Best, Eshel > -John > > -- > Dr. John Haman > Maryland, USA ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-01 11:57 ` Eshel Yaron @ 2023-09-01 13:46 ` John Haman 0 siblings, 0 replies; 9+ messages in thread From: John Haman @ 2023-09-01 13:46 UTC (permalink / raw) To: Eshel Yaron; +Cc: help-gnu-emacs Eshel Yaron <me@eshelyaron.com> writes: > > Here are two patches to make these suggestions concrete, feel free to > apply them as they are, ignore them completely, or anything in between: These are nice! Both patches applied. -- Dr. John Haman Maryland, USA ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-01 1:58 Package critique: modeline for air quality information John Haman 2023-09-01 9:38 ` Eshel Yaron @ 2023-09-01 15:12 ` tpeplt 2023-09-02 21:24 ` Emanuel Berg 2 siblings, 0 replies; 9+ messages in thread From: tpeplt @ 2023-09-01 15:12 UTC (permalink / raw) To: John Haman; +Cc: help-gnu-emacs The following message is a courtesy copy of an article that has been posted to gnu.emacs.help as well. John Haman <mail@johnhaman.org> writes: > I wrote a package that adds local air quality statistics to the > mode-line. Here it is: > > https://github.com/jthaman/air-quality/blob/main/air-quality.el > > If you are so inclined, I'd like some thoughts on the code. It's > short, but I'm trying to get better at Emacs Lisp (at least this week, > while I'm on vacation...) > > -- Some good practices to follow: 1. Byte-compile the source to locate and resolve many types of problems. Byte compilation is available as a command (‘emacs-lisp-byte-compile’) or via the Emacs-Lisp menu when editing a .el file. With your ‘air-quality.el’ (version 0.1 from the git repository), it yields the following message: > In air-quality--get-update: > air-quality.el:125:17: Warning: reference to free variable > ‘url-http-end-of-headers’ 2. Emacs Lisp also has a ‘lint’ available via the Emacs-Lisp menu. This can be helpful, but it can also find spurious (non-existent) problems that are due to its limitations. For ‘air-quality.el’, it yields: > In function air-quality--get-update: > air-quality.el:118:Warning: Reference to unbound symbol: > url-http-end-of-headers > > Linting finished. 3. ‘Checkdoc’ can help identify good practices, such as doc-string standards, so that you (and others) will have usable descriptions of procedures and data structures. It looks as though you have run it on air-quality.el because it gives no warnings or errors. 4. Consider adding the code as an ELPA package that can be installed via the Emacs menu Options/Manage Emacs Packages (or the command ‘list-packages’) once it has reached a level of maturity that you consider acceptable. -- ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-01 1:58 Package critique: modeline for air quality information John Haman 2023-09-01 9:38 ` Eshel Yaron 2023-09-01 15:12 ` tpeplt @ 2023-09-02 21:24 ` Emanuel Berg 2023-09-03 16:42 ` John Haman 2 siblings, 1 reply; 9+ messages in thread From: Emanuel Berg @ 2023-09-02 21:24 UTC (permalink / raw) To: help-gnu-emacs John Haman wrote: > I wrote a package that adds local air quality statistics to > the mode-line. Here it is: > > https://github.com/jthaman/air-quality/blob/main/air-quality.el > > If you are so inclined, I'd like some thoughts on the code. > It's short, but I'm trying to get better at Emacs Lisp (at > least this week, while I'm on vacation...) Can you yank the source here as well or provide a URL to it, that works without JS? But in general, if you byte-compile the source, then native-compile it, and do (checkdoc-current-buffer t) and remove all the warnings, then the Elisp should be good enough and you will improve in the process. -- underground experts united https://dataswamp.org/~incal ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-02 21:24 ` Emanuel Berg @ 2023-09-03 16:42 ` John Haman 2023-09-04 1:15 ` Emanuel Berg 0 siblings, 1 reply; 9+ messages in thread From: John Haman @ 2023-09-03 16:42 UTC (permalink / raw) To: help-gnu-emacs Emanuel Berg <incal@dataswamp.org> writes: > > Can you yank the source here as well or provide a URL to it, > that works without JS? > Here is a link without JS (does your web browser not have JS??) https://raw.githubusercontent.com/jthaman/air-quality/main/air-quality.el -- Dr. John Haman Maryland, USA ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Package critique: modeline for air quality information 2023-09-03 16:42 ` John Haman @ 2023-09-04 1:15 ` Emanuel Berg 0 siblings, 0 replies; 9+ messages in thread From: Emanuel Berg @ 2023-09-04 1:15 UTC (permalink / raw) To: help-gnu-emacs John Haman wrote: >> Can you yank the source here as well or provide a URL to >> it, that works without JS? > > Here is a link without JS (does your web browser not have > JS??) Not Emacs-w3m which I use the most, I of course have Firefox as well but it isn't integrated in the pleasant computer use workflow, if you will. > https://raw.githubusercontent.com/jthaman/air-quality/main/air-quality.el Thanks, looks clean! -- underground experts united https://dataswamp.org/~incal ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-04 1:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-09-01 1:58 Package critique: modeline for air quality information John Haman 2023-09-01 9:38 ` Eshel Yaron 2023-09-01 10:49 ` John Haman 2023-09-01 11:57 ` Eshel Yaron 2023-09-01 13:46 ` John Haman 2023-09-01 15:12 ` tpeplt 2023-09-02 21:24 ` Emanuel Berg 2023-09-03 16:42 ` John Haman 2023-09-04 1:15 ` Emanuel Berg
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).