Subject: Re: bug#68915: Request to include a couple of packages in GNU ELPA

From: Philip Kaludercic <[email-masked]>
Date: Sun, 04 Feb 2024 15:11:39 +0000

Stefan Monnier <[email-masked]> writes:

>>> 2) filechooser.el: https://codeberg.org/rahguzar/filechooser
>>> It implements the backend D-bus methods for xdg filechooser and can be
>>> used to provide an Emacs based file selection interface for applications
>>> such as web browsers which support using xdg desktop portals for this
>>> purpose.

I don't really understand what this package is doing, so my comments are
more superficial, hope they are nevertheless at least a bit useful:

diff --git a/filechooser.el b/filechooser.el
index cc96386..e08f4b1 100644
--- a/filechooser.el
+++ b/filechooser.el
@@ -1,26 +1,27 @@
 ;;; filechooser.el --- An xdg-desktop-portal filechooser -*- lexical-binding: t; -*-
-;;
-;; Copyright (C) 2023 azeem
-;;
+
+;; Copyright (C) 2023 azeem <-- is this you?
+
 ;; Author: rahguzar <[email-masked]>
 ;; Maintainer: rahguzar <[email-masked]>
 ;; Created: May 20, 2023
-;; Modified: May 20, 2023
+;; Modified: May 20, 2023 <-- either use time-stamp or remove this please
 ;; Version: 0.0.1
 ;; Keywords: convenience files tools unix
 ;; Homepage: https://codeberg.org/rahguzar/filechooser
 ;; Package-Requires: ((emacs "28.1") (compat "29.1"))
-;;
+
 ;; This file is not part of GNU Emacs.
-;;
+
 ;;; Commentary:
-;; An implementation of xdg-desktop-portal filechooser in Emacs. This allows
+
+;; An implementation of xdg-desktop-portal filechooser in Emacs.  This allows
 ;; for choosing files in applications like firefox (with GTK_USE_PORTAL set)
-;; from an Emacs frame. The default is to use `read-file-name' for choosing
+;; from an Emacs frame.  The default is to use `read-file-name' for choosing
 ;; a single file and a pair of Dired buffers for choosing multiple files.
-;;
-;;
+
 ;;; Code:
+
 (require 'compat)
 (require 'dbus)
 (require 'xdg)
@@ -49,25 +50,27 @@
 
 (defvar filechooser-multiple-selection-map
   (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map filechooser-mininuffer-map)
     (define-key map (kbd "M-TAB") #'filechooser-multiple-continue)
     (define-key map (kbd "M-RET") #'filechooser-multiple-finalize-current-selection)
     (define-key map (kbd "M-a") #'filechooser-multiple-select-all)
-    (make-composed-keymap map filechooser-mininuffer-map)))
+    map))
 
 ;;;; Custom variables
-(defgroup filechooser nil
+(defgroup filechooser nil		;this is a duplicate!
   "An xdg-desktop-portal filechooser."
   :link '(url-link :tag "Homepage" "https://codeberg.org/rahguzar/filechooser")
   :group 'files)
 
 (defcustom filechooser-save-existing-files 'uniquify
   "Determines behavior when attempting to save an existing file FILENAME.
-If it is symbol `yes-or-no-p', `yes-or-no-p' is used to confirm if the file
-should be overwritten. If it is the symbol `y-or-n-p', `y-or-n-p' is used to
-prompt. In both cases if a the answer is negative, the file selection is started
-again. If it is the symbol `uniquify', the FILENAME is made unique by appedning
--N to it where N is a positive number. If it is a function, it is called with
-FILENAME and the return value is used as the filename."
+If it is symbol `yes-or-no-p', `yes-or-no-p' is used to confirm if the
+file should be overwritten.  If it is the symbol `y-or-n-p', `y-or-n-p'
+is used to prompt.  In both cases if a the answer is negative, the file
+selection is started again.  If it is the symbol `uniquify', the
+FILENAME is made unique by appedning -N to it where N is a positive
+number.  If it is a function, it is called with FILENAME and the return
+value is used as the filename."
   :type '(choice
           (const :tag "Uniquify" uniquify)
           (const :tag "Prompt Yes/No" yes-or-no-p)
@@ -80,8 +83,8 @@ If it is nil the selected frame is used instead."
   :type 'boolean)
 
 (defcustom filechooser-filters `(("Directories" filechooser-file-directory-p . t)
-                              ("Elisp files" ,(rx ".el" eos))
-                              ("Not dot files" ,(rx bos (not ?.))))
+				 ("Elisp files" ,(rx ".el" eos))
+				 ("Not dot files" ,(rx bos (not ?.))))
   "An alist of (NAME FILTER . BOOL).
 NAME should describe the filter which can either be a regexp
 or else a predicate function which takes a filename as argument.
@@ -89,8 +92,8 @@ If BOOL is non-nil filter is active by default otherwise it is inactive."
   :type '(alist :key-type (string :tag "Name")
                 :value-type
                 (cons :tag "Value:"
-                  (choice :tag "Filter" regexp function)
-                  (boolean :tag "Default"))))
+                      (choice :tag "Filter" regexp function)
+                      (boolean :tag "Default"))))
 
 (defcustom filechooser-choose-file #'filechooser-read-file-name
   "Function used to choose a single file.
@@ -114,7 +117,7 @@ It should have the same calling convention as
   "The key that is used to exit minibuffer to do completion.
 I.e. the key that binds the equivalent of `exit-minibuffer' for the completion
 UI of choice: usually RET."
-  :type 'key)
+  :type 'key)				;this is not available for Emacs 27
 
 ;;;; Others
 (defvar filechooser-current-operation nil
@@ -122,12 +125,12 @@ UI of choice: usually RET."
 
 ;;;; Internal Variables
 (defvar filechooser--filters nil)
-(defvar filechooser--selection (list (make-temp-file "filechooser-selection-" t)))
+(defvar filechooser--selection (list (make-temp-file "filechooser-selection-" t))) ;do you really want to evaluate a side effect here?
 (defvar filechooser--multiple-selection nil)
 
 ;;; Filters
 (defun filechooser--filters-group-fn (cand transform)
-  "Group function for selecting filters. CAND TRANSFORM."
+  "Group function for selecting filters.  CAND TRANSFORM." ;please document CAND and TRANSFORM
   (if transform
       cand
     (if (cdr (alist-get cand filechooser--filters nil nil #'equal))
@@ -136,9 +139,9 @@ UI of choice: usually RET."
 
 (defun filechooser-file-directory-p (name)
   "Return non-nil if NAME is an existing directory."
-    (file-directory-p (if (derived-mode-p 'dired-mode)
-                          (expand-file-name name (dired-current-directory))
-                        name)))
+  (file-directory-p (if (derived-mode-p 'dired-mode)
+                        (expand-file-name name (dired-current-directory))
+                      name)))
 
 (defun filechooser-toggle-filter (arg)
   "Toggle a filter.
@@ -170,13 +173,12 @@ With prefix ARG toggle multiple filters using `completing-read-multiple'."
         (current (caar (alist-get "current_filter" opts nil nil #'equal)))
         (regex-filters)
         (glob-to-regexp (lambda (cell) (if (eq  0 (car cell))
-                                      `(regexp ,(dired-glob-regexp (nth 1 cell)))
-                                    ""))))
+					   `(regexp ,(dired-glob-regexp (nth 1 cell)))
+					 ""))))
     (unless (alist-get (car current) filters nil nil #'equal)
       (when current (push current filters)))
     (pcase-dolist (`(,name ,globs) filters)
-      (push (list name
-                  (rx-to-string `(or ,@(mapcar glob-to-regexp globs))))
+      (push (list name (mapconcat #'wildcard-to-regexp "\\|" globs))
             regex-filters))
     (when (and current (not (caar (alist-get "directory" opts nil nil #'equal))))
       (cl-callf not (cdr (alist-get (car current) regex-filters nil nil #'equal))))
@@ -187,9 +189,12 @@ With prefix ARG toggle multiple filters using `completing-read-multiple'."
   (lambda (name)
     (catch 'match
       (dolist (filter filters)
-        (when (if (stringp filter)
-                  (string-match filter name)
-                (funcall filter name))
+        (when (cond
+	       ((stringp filter)
+                (string-match filter name)) ;or string-match-p?
+               ((functionp filter)
+		(funcall filter name))
+	       ((error "Unknown filter %S" filter)))
           (throw 'match t))))))
 
 ;;; Utility definitions
@@ -199,18 +204,18 @@ MINIBUFFER is the value of minibuffer frame paramter."
   (declare (indent 1))
   (let ((framevar (make-symbol "frame")))
     `(let ((minibuffer-completing-file-name ,(eq minibuffer 'only)))
-      (if filechooser-use-popup-frame
-         (let ((,framevar (make-frame '((name . ,(if (eq minibuffer 'only)
-                                                     "filechooser-miniframe"
-                                                   "filechooser-frame"))
-                                        (minibuffer . ,minibuffer)))))
-           (unwind-protect
-               (with-demoted-errors "%S"
-                 (with-selected-frame ,framevar
-                   ,@body))
-             (delete-frame ,framevar 'force)))
-       (with-demoted-errors "%S"
-         ,@body)))))
+       (if filechooser-use-popup-frame
+           (let ((,framevar (make-frame '((name . ,(if (eq minibuffer 'only)
+                                                       "filechooser-miniframe"
+                                                     "filechooser-frame"))
+                                          (minibuffer . ,minibuffer)))))
+             (unwind-protect
+		 (with-demoted-errors "%S"
+                   (with-selected-frame ,framevar
+                     ,@body))
+               (delete-frame ,framevar 'force)))
+	 (with-demoted-errors "%S"
+           ,@body)))))
 
 (defun filechooser-abort ()
   "Abort the file selection."
@@ -244,7 +249,7 @@ See Info node `(elisp) Programmed Completion' for STR, PRED and ACTION."
   (pcase action
     ('t (let ((dir (or (file-name-directory str) default-directory)))
           (cl-callf2 mapcar (lambda (sel) (cons (car sel)
-                                           (file-relative-name (car sel) dir)))
+						(file-relative-name (car sel) dir)))
                      filechooser--multiple-selection)
           (append (mapcar #'cdr filechooser--multiple-selection)
                   (completion-file-name-table str pred t))))
@@ -255,12 +260,12 @@ See Info node `(elisp) Programmed Completion' for STR, PRED and ACTION."
 
 (defun filechooser--read-file-name-1 (prompt &optional mustmatch filters dir default)
   "Read a filename with PROMPT and predicate made from FILTERS.
-MUSTMATCH and DIR are as in `read-file-name'. DEFAULT is the default filename.
+MUSTMATCH and DIR are as in `read-file-name'.  DEFAULT is the default filename.
 If MULTIPLE is non-nil `completing-read-multiple' is used."
   (catch 'continue
     (minibuffer-with-setup-hook
         (lambda () (use-local-map (make-composed-keymap filechooser-mininuffer-map
-                                                   (current-local-map)))
+							(current-local-map)))
           (when dir (setq default-directory dir)))
       (read-file-name
        prompt dir default mustmatch nil
@@ -271,25 +276,25 @@ If MULTIPLE is non-nil `completing-read-multiple' is used."
 DIR is the directory to use if a new file name needs to be choosen and FILTERS
 are the filters to use in that case."
   (pcase filechooser-save-existing-files
-        ('uniquify
-         (let ((n 1)
-               (name (file-name-sans-extension filename))
-               (ext (file-name-extension filename t)))
-           (while (file-exists-p (format "%s-%s%s" name n ext))
-             (cl-incf n))
-           (format "%s-%s%s" name n ext)))
-        ((or 'yes-or-no-p 'y-or-n-p)
-         (if (funcall filechooser-save-existing-files
-                      (format "File %s exists. Overwrite?" filename))
-             filename
-           (filechooser--read-file-name "Choose a new file name: "
-                                        nil filters dir
-                                        (file-relative-name filename dir))))
-        (_ (funcall filechooser-save-existing-files filename))))
+    ('uniquify
+     (let ((n 1)
+           (name (file-name-sans-extension filename))
+           (ext (file-name-extension filename t)))
+       (while (file-exists-p (format "%s-%s%s" name n ext))
+         (cl-incf n))
+       (format "%s-%s%s" name n ext)))
+    ((or 'yes-or-no-p 'y-or-n-p)
+     (if (funcall filechooser-save-existing-files
+                  (format "File %s exists. Overwrite?" filename))
+         filename
+       (filechooser--read-file-name "Choose a new file name: "
+                                    nil filters dir
+                                    (file-relative-name filename dir))))
+    (_ (funcall filechooser-save-existing-files filename))))
 
 (defun filechooser--read-file-name (prompt &optional mustmatch filters dir default)
   "Read a filename with PROMPT and predicate made from FILTERS.
-MUSTMATCH and DIR are as in `read-file-name'. DEFAULT is the default filename.
+MUSTMATCH and DIR are as in `read-file-name'.  DEFAULT is the default filename.
 If MULTIPLE is non-nil `completing-read-multiple' is used."
   (setq filechooser--filters (cl-delete-duplicates
                               (append filechooser-filters filters)
@@ -311,12 +316,12 @@ If MULTIPLE is non-nil `completing-read-multiple' is used."
   "Read a file name.
 If `filechooser-use-popup-frame' is non-nil a new minibuffer only popup frame
 is used, othewise the selected frame is used.
-PROMPT is the minibuffer prompt. MUSTMATCH and DIR are as in `read-file-name'.
-FILTERS take the same form as elements of `filechooser-filters'. Only those
+PROMPT is the minibuffer prompt.  MUSTMATCH and DIR are as in `read-file-name'.
+FILTERS take the same form as elements of `filechooser-filters'.  Only those
 files which satisfy one of the active filters from FILTERS or
 `filechooser-filters' are presented for completions."
   (filechooser--maybe-with-new-frame only
-    (filechooser--read-file-name prompt mustmatch filters dir default)))
+				     (filechooser--read-file-name prompt mustmatch filters dir default)))
 
 (defun filechooser-multiple-continue ()
   "Select current file and exit multiple file selection."
@@ -362,8 +367,8 @@ MAP contains additional key bindigs."
             (catch 'continue
               (minibuffer-with-setup-hook
                   (lambda () (use-local-map (make-composed-keymap
-                                        filechooser-multiple-selection-map
-                                        (current-local-map))))
+                                             filechooser-multiple-selection-map
+                                             (current-local-map))))
                 (completing-read prompt #'filechooser--multiple-loop-table
                                  (filechooser--filters-predicate filters) t
                                  (abbreviate-file-name dir)
@@ -374,8 +379,8 @@ MAP contains additional key bindigs."
   "Read multiple file names using `completing-read-multiple'.
 If `filechooser-use-popup-frame' is non-nil a new minibuffer only popup frame
 is used, othewise the selected frame is used.
-PROMPT is the minibuffer prompt. DIR is the directory where selection starts.
-FILTERS take the same form as elements of `filechooser-filters'. Only those
+PROMPT is the minibuffer prompt.  DIR is the directory where selection starts.
+FILTERS take the same form as elements of `filechooser-filters'.  Only those
 files which satisfy one of the active filters from FILTERS or
 `filechooser-filters' are presented for completions."
   (setq filechooser--filters
@@ -386,35 +391,38 @@ files which satisfy one of the active filters from FILTERS or
              (expand-file-name (or dir default-directory))))
   (let (selected filechooser--multiple-selection)
     (filechooser--maybe-with-new-frame only
-      (while (setq dir
-                   (catch 'done
-                     (setq selected (expand-file-name
-                                     (filechooser--multiple-read-file-name
-                                      prompt dir)))
-                     (cl-callf not
-                         (alist-get selected filechooser--multiple-selection
-                                    nil t #'equal))
-                     (when (eq this-command 'filechooser-multiple-continue)
-                       (expand-file-name (file-name-directory selected))))))
-      (nreverse (mapcar #'car filechooser--multiple-selection)))))
+				       ;; perhaps rewrite this to
+				       ;; improve the default
+				       ;; indentation
+				       (while (setq dir
+						    (catch 'done
+						      (setq selected (expand-file-name
+								      (filechooser--multiple-read-file-name
+								       prompt dir)))
+						      (cl-callf not
+							  (alist-get selected filechooser--multiple-selection
+								     nil t #'equal))
+						      (when (eq this-command 'filechooser-multiple-continue)
+							(expand-file-name (file-name-directory selected))))))
+				       (nreverse (mapcar #'car filechooser--multiple-selection)))))
 
 (defun filechooser-save-files (prompt &optional dir files)
   "Read a directory name to save FILES in it.
 If `filechooser-use-popup-frame' is non-nil a new minibuffer only popup frame
-is used, othewise the selected frame is used. PROMPT and DIR are as in
+is used, othewise the selected frame is used.  PROMPT and DIR are as in
 `read-directory-name'."
   (filechooser--maybe-with-new-frame only
-    (when-let ((save-dir (read-directory-name prompt dir))
-               (names nil))
-      (make-directory save-dir t)
-      (catch 'abort
-        (dolist (file files)
-          (push (if (file-exists-p (expand-file-name file save-dir))
-                    (or (filechooser--handle-exisiting-file file save-dir)
-                        (throw 'abort nil))
-                  (expand-file-name file save-dir))
-                names))
-        names))))
+				     (when-let ((save-dir (read-directory-name prompt dir))
+						(names nil))
+				       (make-directory save-dir t)
+				       (catch 'abort
+					 (dolist (file files)
+					   (push (if (file-exists-p (expand-file-name file save-dir))
+						     (or (filechooser--handle-exisiting-file file save-dir)
+							 (throw 'abort nil))
+						   (expand-file-name file save-dir))
+						 names))
+					 names))))
 
 ;;; Dired based selection
 (let (marked unmarked timer)
@@ -449,14 +457,14 @@ is used, othewise the selected frame is used. PROMPT and DIR are as in
 (defun filechooser-dired (&optional dir filters)
   "Select some files using Dired.
 Running this command pops a Dired for directory DIR, and enters a recursive
-editing session. FILTERS are in the format of `filechooser-filters'."
+editing session.  FILTERS are in the format of `filechooser-filters'."
   (let ((overriding-map `((t . ,filechooser-dired-overriding-map)))
         (apply-filters (lambda (_) (when (and (derived-mode-p 'dired-mode)
-                                         (not (eq (current-buffer) (cdr filechooser--selection)))
-                                         (not (memq 'filechooser--dired-jit-filter jit-lock-functions)))
-                                (add-hook 'jit-lock-functions #'filechooser--dired-jit-filter 95 t)
-                                (jit-lock-mode t)
-                                (add-to-invisibility-spec 'filechooser-filter))))
+                                              (not (eq (current-buffer) (cdr filechooser--selection)))
+                                              (not (memq 'filechooser--dired-jit-filter jit-lock-functions)))
+                                     (add-hook 'jit-lock-functions #'filechooser--dired-jit-filter 95 t)
+                                     (jit-lock-mode t)
+                                     (add-to-invisibility-spec 'filechooser-filter))))
         (selection-buffer (progn (setcdr filechooser--selection nil)
                                  (dired-noselect filechooser--selection))))
     (unwind-protect
@@ -528,8 +536,8 @@ editing session. FILTERS are in the format of `filechooser-filters'."
 (defun filechooser-with-dired (_prompt &optional dir filters)
   "Select some files using Dired.
 If `filechooser-use-popup-frame' is non-nil a new frame is used for selection,
-otherwise selected frame is used. DIR is the directory for initial Dired
-buffer. FILTERS are used to restrict selection to a subset of files."
+otherwise selected frame is used.  DIR is the directory for initial Dired
+buffer.  FILTERS are used to restrict selection to a subset of files."
   (filechooser--maybe-with-new-frame t (filechooser-dired dir filters)))
 
 ;;; Method handlers

-- 
Philip Kaludercic