Re: [ELPA] Add theme-buffet package

[Philip Kaludercic <philipk@posteo.net>]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

Bruno Boal <egomet@bboal.com> writes:

> Hello everyone,
>
> I would like you to consider including my package in the GNU ELPA.
> I've just filled out the copyright assignment form, and in the meantime
> I'm sending the proposed changes below for your consideration.

Here are a few comments in form of a diff (this is not a patch that you
should apply, but just some suggestions, hints and ideas that can be
discussed):


[-- Attachment #2: Type: text/plain, Size: 17397 bytes --]

diff --git a/theme-buffet.el b/theme-buffet.el
index 08934cd..e65cc0d 100644
--- a/theme-buffet.el
+++ b/theme-buffet.el
@@ -7,7 +7,10 @@
 ;; Maintainer: Theme-Buffet Development <~bboal/general-issues@lists.sr.ht>
 ;; URL: https://git.sr.ht/~bboal/theme-buffet
 ;; Version: 0.0.0
-;; Package-Requires: ((emacs "29.1"))
+;; Package-Requires: ((emacs "26.1"))
+
+;; According to package-lint, the newest function you depend on is
+;; from Emacs 29 is `take', If you want to depend on Compat, you could lower the
 
 ;; This file is NOT part of GNU Emacs.
 
@@ -29,7 +32,7 @@
 ;; Theme-Buffet lets the user specify different time periods of the day and for
 ;; each period, a list of preferred themes to be randomly loaded accordingly.
 ;; To install you just have to clone the repo from the url, add the path to the
-;; 'load-path variable and then require the library. Here's an example, for
+;; 'load-path variable and then require the library.  Here's an example, for
 ;; those who still love the Emacs 28 or earlier way of doing things:
 ;;
 ;;    # In the terminal
@@ -44,7 +47,7 @@
 ;;    (package-vc-install "https://git.sr.ht/~bboal/theme-buffet")
 ;;
 ;;
-;; There are two templates preconfigured available for usage. One enabled by
+;; There are two templates preconfigured available for usage.  One enabled by
 ;; default, with the standard themes that come with vanilla Emacs; the other
 ;; more fancier, can be easily enabled by evaluating the following:
 ;;
@@ -52,39 +55,38 @@
 ;;
 ;; The binding above will set the themes to be either Modus or Ef, authored by
 ;; Protesilaos Stavrou <https://git.sr.ht/~protesilaos>, distributed across six
-;; periods of the day (night, twilight, morning, day, afternoon and evening). The
+;; periods of the day (night, twilight, morning, day, afternoon and evening).  The
 ;; library will require the aforementioned package, installing if
-;; necessary. Finally to start using Theme-Buffet, evaluate:
+;; necessary.  Finally to start using Theme-Buffet, evaluate:
 ;;
 ;;    (theme-buffet-mode 1)
 ;;
 ;;
 ;; Following the appanage way of Emacs, both the names and number of themes and
-;; time periods can be freely changed while mantaining the same structure. There
+;; time periods can be freely changed while mantaining the same structure.  There
 ;; is also a time-offset that can be set by the user to match a specific
-;; time-zone/personal preference. E.g
+;; time-zone/personal preference.  E.g
 ;;
 ;;    (setq theme-buffet-time-offset 2)
 ;;
-;; All this can be achieved by tweaking `theme-buffet-end-user'. For
+;; All this can be achieved by tweaking `theme-buffet-end-user'.  For
 ;; inspiration, take a look at `theme-buffet--modus-ef' which is used when
 ;; setting `theme-buffet-menu' to 'modus-ef like demonstrated above.
 ;;
 ;;
 ;; Disclaimer from Bruno Boal to the reader: This package was produced during
 ;; my learning sessions with Protesilaos "Prot" Stavrou and improved as
-;; homework. Most of the credit goes to him, the mistakes you may find are my
-;; own. Personally, despite the disadvantages and advantages of not being a
+;; homework.  Most of the credit goes to him, the mistakes you may find are my
+;; own.  Personally, despite the disadvantages and advantages of not being a
 ;; professional programmer, it is essential for me to always have fun and
-;; enjoyment during learning and programming. In this respect, mission
-;; accomplished, a big "thank you!" to my mentor. Also, keep in mind at least
+;; enjoyment during learning and programming.  In this respect, mission
+;; accomplished, a big "thank you!" to my mentor.  Also, keep in mind at least
 ;; two things - the fact that this package, like many others before it, has its
 ;; genesis in a collective effort, with didatic purposes and personal use in
 ;; mind, but also that future improvements could and should come from people
 ;; like you, a user of free software.
 ;;
 ;; Happy hacking!
-;;
 
 ;;; Code:
 
@@ -92,22 +94,22 @@
 (defgroup theme-buffet nil
   "Time based theme switcher.
 Assortment of preference based themes available for consumption according to
-the time of the day. A true theme feast for the eyes..."
+the time of the day.  A true theme feast for the eyes..."
   :group 'faces)
 
-
-(defun theme-buffet--get-themes(&optional plist-usage)
+(defun theme-buffet--get-themes (&optional plist-usage)
   "Get themes from default Emacs build directory and `custom-theme-load-path'.
 Return list for usage in `theme-buffet-menu' type options if PLIST-USAGE is
 non-nil."
+  ;; Why not `custom-available-themes'?
   (let* ((default (expand-file-name "themes/" data-directory))
          (custom  (butlast custom-theme-load-path))
          (themes-dirs (cons default custom))
-         (themes (flatten-tree
-                  (mapcar (lambda (f)
-                            (if (symbolp f) (setq f (symbol-value f)))
-                            (directory-files f nil ".*-theme\\.el\\'"))
-                          themes-dirs))))
+         (themes (mapcan (lambda (f)
+                           (directory-files
+			    (if (symbolp f) (symbol-value f) f)
+			    nil ".*-theme\\.el\\'"))
+                         themes-dirs)))
     (mapcar (lambda (theme)
               (let ((symbol-list (intern
                                   (string-trim-right theme "-theme\\.el"))))
@@ -116,8 +118,6 @@ non-nil."
 
 (defvar theme-buffet--const-themes (theme-buffet--get-themes t))
 
-
-
 (defconst theme-buffet--built-in
   '(:night     (wheatgrass manoj-dark modus-vivendi)
     :morning   (adwaita whiteboard leuven modus-operandi tango dichromacy tsdh-light)
@@ -171,20 +171,20 @@ For those who just don't have the time and want the best.")
     :afternoon (leuven-dark tango-dark tsdh-dark misterioso)
     :evening   (deeper-blue wombat))
   "Associate day periods with list of themes.
-Each association is of the form `:KEYWORD (THEMES)' where :KEYWORD is one among
-:dark, :twilight, :dawn, etc, and (THEMES), a list of existent themes. Prefilled
-with Emacs default themes as an example to change by the user."
+Each association is of the form `:KEYWORD (THEMES)' where
+:KEYWORD is one among :dark, :twilight, :dawn, etc, and (THEMES),
+a list of existent themes.  Prefilled with Emacs default themes
+as an example to change by the user."
   :type `(plist
           :options
-              (((const :tag "Darkness of the night" :night)
-                (repeat (choice symbol ,@theme-buffet--const-themes)))
-               ((const :tag "Bright sun is up" :morning)
-                (repeat (choice symbol ,@theme-buffet--const-themes)))
-               ((const :tag "Perhaps a clouded afternoon" :afternoon)
-                (repeat (choice symbol ,@theme-buffet--const-themes)))
-               ((const :tag "Close to the sunset" :evening)
-                (repeat (choice symbol ,@theme-buffet--const-themes))))))
-
+          (((const :tag "Darkness of the night" :night)
+            (repeat (choice symbol ,@theme-buffet--const-themes)))
+           ((const :tag "Bright sun is up" :morning)
+            (repeat (choice symbol ,@theme-buffet--const-themes)))
+           ((const :tag "Perhaps a clouded afternoon" :afternoon)
+            (repeat (choice symbol ,@theme-buffet--const-themes)))
+           ((const :tag "Close to the sunset" :evening)
+            (repeat (choice symbol ,@theme-buffet--const-themes))))))
 
 (defcustom theme-buffet-menu 'built-in
   "Define which property list to use when selecting the theme list."
@@ -192,49 +192,37 @@ with Emacs default themes as an example to change by the user."
                  (const :tag "Modus and Ef themes" modus-ef)
                  (const :tag "User specified themes" end-user)))
 
-
 (defun theme-buffet--selected-menu ()
   "Return property list based on given MENU."
-  (cond
-   ((eq theme-buffet-menu 'built-in)
-    theme-buffet--built-in)
-   ((eq theme-buffet-menu 'modus-ef)
-    theme-buffet--modus-ef)
-   ((eq theme-buffet-menu 'end-user)
-    theme-buffet--end-user)
-   (t
-    nil)))
-
+  (pcase-exhaustive theme-buffet-menu
+    ('built-in theme-buffet--built-in)
+    ('modus-ef theme-buffet--modus-ef)
+    ('end-user theme-buffet--end-user)))
 
-(defun theme-buffet--hours-secs(hours)
+(defun theme-buffet--hours-secs (hours)
   "Number of seconds in HOURS."
   (* hours 60 60))
 
-
 (defconst theme-buffet--secs-in-day
   (theme-buffet--hours-secs 24)
   "Number of seconds in a day.")
 
-
-(defun theme-buffet--keywords()
+(defun theme-buffet--keywords ()
   "Get the name of the keywords defining the day periods."
   (let ((selected-menu (theme-buffet--selected-menu)))
     (if (plistp selected-menu)
         (seq-filter #'keywordp selected-menu)
-      (user-error "The Theme-Buffet Chef cannot work with your supplied themes. Check `theme-buffet-menu'"))))
-
+      (user-error "The Theme-Buffet Chef cannot work with your supplied themes.  Check `theme-buffet-menu'"))))
 
-(defun theme-buffet--periods()
+(defun theme-buffet--periods ()
   "Get the number of keywords that define the day periods."
   (length (theme-buffet--keywords)))
 
-
-(defun theme-buffet--interval()
+(defun theme-buffet--interval ()
   "Get the number of seconds that each given time period should remain active."
   (/ theme-buffet--secs-in-day (theme-buffet--periods)))
 
-
-(defun theme-buffet--get-time()
+(defun theme-buffet--get-time ()
   "Get the `current-time' in seconds."
   (let ((time-smh (take 3 (decode-time)))
         seconds)
@@ -242,8 +230,7 @@ with Emacs default themes as an example to change by the user."
       (setq seconds (cons (pop time-smh) seconds)
             time-smh (mapcar (lambda (n) (* 60 n))
                              time-smh)))
-    (apply #'+ seconds)))
-
+    (seq-reduce #'+ seconds 0)))
 
 (defun theme-buffet--natnum-from-to (start end &optional step)
   "Create a list for applying in defcustom's type choice customization.
@@ -254,14 +241,12 @@ END-STEP) (const END))"
             (list 'const x))
           (number-sequence start end step)))
 
-
 (defcustom theme-buffet-time-offset 0
   "Added time in HOURS (integer number) to shift the day periods.
 Used for compensate winter/summer times or specific weather situations."
   :type `(choice ,@(theme-buffet--natnum-from-to -12 12)))
 
-
-(defun theme-buffet--get-offset()
+(defun theme-buffet--get-offset ()
   "Error checking for `theme-buffet-time-offset' variable.
 Has to be an integer number and no greater than 12h in absolute value"
   (cond
@@ -273,21 +258,18 @@ Has to be an integer number and no greater than 12h in absolute value"
    (t
     (theme-buffet--hours-secs theme-buffet-time-offset))))
 
-
-(defun theme-buffet--current-period()
+(defun theme-buffet--current-period ()
   "Get the current period reference the number of keywords in `theme-buffet'."
   (let ((offset (mod (+ (theme-buffet--get-time)
                         (theme-buffet--get-offset))
                      theme-buffet--secs-in-day)))
     (ceiling offset (theme-buffet--interval))))
 
-
-(defun theme-buffet--get-period-keyword()
+(defun theme-buffet--get-period-keyword ()
   "Get the keyword of the current period as specified in `theme-buffet'."
   (nth (1- (theme-buffet--current-period)) (theme-buffet--keywords)))
 
-
-(defun theme-buffet--reload-theme(chosen-theme &optional added-message)
+(defun theme-buffet--reload-theme (chosen-theme &optional added-message)
   "Load CHOSEN-THEME after disabling the current one.
 An additional ADDED-MESSAGE can be appended to the original string for added
 information."
@@ -298,20 +280,18 @@ information."
     (load-theme chosen-theme :no-confirm)
     (message "%s `%s'" message chosen-theme)))
 
-
-(defun theme-buffet--get-theme-list(period)
+(defun theme-buffet--get-theme-list (period)
   "Get list of themes of PERIOD, excluding the current if more are available."
   (when-let ((selected-menu (theme-buffet--selected-menu))
              (theme-list (plist-get selected-menu period)))
     (or (remq (car custom-enabled-themes) theme-list)
         theme-list)))
 
-
-(defun theme-buffet--load-random(period)
+(defun theme-buffet--load-random (period)
   "Load random theme according to PERIOD.
 
 Omit current theme if it's not the only pertaining to the list of the
-corresponding period. Being this the case, the same theme shall be served.
+corresponding period.  Being this the case, the same theme shall be served.
 
 An error message will appear if the theme is not available to load through
 `load-theme'."
@@ -322,12 +302,10 @@ An error message will appear if the theme is not available to load through
     (user-error "Theme-Buffet Chef says `%s' is not known or installed!"
                 chosen-theme)))
 
-
-
 (defvar theme-buffet-theme-history nil
   "Theme-Buffet period history.")
 
-(defun theme-buffet--theme-prompt()
+(defun theme-buffet--theme-prompt ()
   "Prompt the user the theme to choose for the present period."
   (let ((prompt "From current period choose a theme: ")
         (collection (theme-buffet--get-theme-list
@@ -336,19 +314,17 @@ An error message will appear if the theme is not available to load through
     (completing-read prompt collection nil t nil history-var)))
 
 ;;;###autoload
-(defun theme-buffet-a-la-carte()
+(defun theme-buffet-a-la-carte ()
   "Prompt user for theme according to the current period of the day."
   (declare (interactive-only t))
   (interactive)
   (let ((chosen-theme (intern (theme-buffet--theme-prompt))))
     (theme-buffet--reload-theme chosen-theme "as per your desires. Enjoy..." )))
 
-
-
 (defvar theme-buffet-period-history nil
   "Theme-Buffet period history.")
 
-(defun theme-buffet--period-prompt()
+(defun theme-buffet--period-prompt ()
   "Prompt user for the day period from the list of periods."
   (let ((prompt "Choose a period of the day: ")
         (collection (theme-buffet--keywords))
@@ -356,28 +332,24 @@ An error message will appear if the theme is not available to load through
     (completing-read prompt collection nil t nil history-var)))
 
 ;;;###autoload
-(defun theme-buffet-order-other-period()
+(defun theme-buffet-order-other-period ()
   "Interactively load a random theme from the current day period."
   (declare (interactive-only t))
   (interactive)
   (let ((period (intern (theme-buffet--period-prompt))))
     (theme-buffet--load-random period)))
 
-
 ;;;###autoload
-(defun theme-buffet-anything-goes()
+(defun theme-buffet-anything-goes ()
   "Interactively load an existing random theme."
   (declare (interactive-only t))
   (interactive)
   (theme-buffet--reload-theme (seq-random-elt (theme-buffet--get-themes))
                               "as a suprise"))
 
-
-
 (defvar theme-buffet-user-timers-history nil
   "Theme-Buffet user timers history.")
 
-
 ;;;; Period timer
 (defvar theme-buffet-timer-periods nil
   "Timer that calls Theme-Buffet's Chef into the kitchen.")
@@ -390,14 +362,19 @@ An error message will appear if the theme is not available to load through
 (defvar theme-buffet-timer-mins nil
   "Timer that calls another Theme-Buffet's Sous-Chef into the kitchen.")
 
-
-(defun theme-buffet--free-timer(timer-obj)
+(defun theme-buffet--free-timer (timer-obj)
   "Cancel and set to nil the timer TIMER-OBJ."
   (when-let (((boundp timer-obj))
              (obj (symbol-value timer-obj)))
     (cancel-timer obj)
     (set timer-obj nil)))
 
+;; I am not too thrilled about this section.  It would be nice if
+;; instead of generating a macro you could use construct a lambda
+;; expression that you bind to the intended symbol using `defalias'.
+;; A yet simpler approach would be to define a generic, internal
+;; function that written-out functions would directly invoke, instead
+;; of manually writing autoload cookies.
 
 (defmacro theme-buffet--define-timer(units)
   "Define interactive functions to set timer in UNITS.
@@ -431,14 +408,14 @@ naming."
                                        (theme-buffet--get-period-keyword)))
            (message "Theme-Buffet Sous-Chef is rushing into the kitchen...")))))))
 
+
+
 ;;;###autoload (autoload 'theme-buffet-timer-mins "theme-buffet")
 (theme-buffet--define-timer mins)   ; (theme-buffet-timer-mins n)
 ;;;###autoload (autoload 'theme-buffet-timer-hours "theme-buffet")
 (theme-buffet--define-timer hours)  ; (theme-buffet-timer-hours n)
 
-
-
-(defmacro theme-buffet--define-menu-defuns(menu)
+(defmacro theme-buffet--define-menu-defuns (menu)
   "Define interactive functions to choose property list with themes to use.
 The timer is clean, the chosen MENU is set with it's corresponding keywords."
   (let* ((doc-built-in "Built-in Emacs themes. If you like minimalism and standard suits your needs.")
@@ -470,17 +447,13 @@ opinion.")
 ;;;###autoload (autoload 'theme-buffet-end-user "theme-buffet")
 (theme-buffet--define-menu-defuns end-user)  ; (theme-buffet-end-user)
 
-
-
 ;;;###autoload
 (define-minor-mode theme-buffet-mode
   "Theme-Buffet serves your preferred themes according to the time of day.
-You eyes will thank you. Or not...
+You eyes will thank you.  Or not...
 
 The preference for the themes is specified in the `theme-buffet-menu'"
-  :init-value nil
   :global t
-  :keymap nil
   (if theme-buffet-mode
       ;; 2023-11-20 FIXME => The `unless' below is because `theme-buffet-mode' is
       ;; called every time the timer runs without an explanation.

[-- Attachment #3: Type: text/plain, Size: 908 bytes --]


>
> diff --git a/elpa-packages b/elpa-packages
> index 608d8a353b..de5e15df48 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -683,6 +683,8 @@
>    :news "CHANGELOG.org"
>    :ignored-files ("LICENSE"))
>   (test-simple         	:url "https://github.com/rocky/emacs-test-simple")
> + (theme-buffet		:url "https://git.sr.ht/~bboal/theme-buffet"
> +  :readme "README.md")

Are you sure you want this, the contents of your README appear to be
identical to your "Commentary" section (something I don't recommend in
general, I see a README as something that interests people who have
checked out your repository, while the Commentary should describe the
intention and the entry-points of your package).

>   (timerfunctions	:url nil)
>   (tiny			:url "https://github.com/abo-abo/tiny")
>   (tmr			:url "https://git.sr.ht/~protesilaos/tmr"
>
>
> Let me know what you think.
>
> Best regards,
> Bruno Boal