Re: Package critique: modeline for air quality information

[Eshel Yaron <me@eshelyaron.com>]

[-- 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