Re: Package critique: modeline for air quality information

[Eshel Yaron]

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:

0001-Refine-defcustom-types.patch
Description: Text Data

>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

Best,

Eshel

> -John
>
> --
> Dr. John Haman
> Maryland, USA