Package: elpa;
Reported by: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date: Sat, 3 Feb 2024 22:32:01 UTC
Severity: normal
Message #23 received at 68915 <at> debbugs.gnu.org (full text, mbox):
From: Rahguzar <rahguzar <at> zohomail.eu> To: Philip Kaludercic <philipk <at> posteo.net> Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 68915 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com> Subject: Re: bug#68915: Request to include a couple of packages in GNU ELPA Date: Sun, 04 Feb 2024 21:57:44 +0100
Hi Philip,
Thanks again for these.
I will make most of the stylistic changes but I would like to not mix
tabs and spaces for the code I want to maintain. Other than that some
responses to your questions are below:
Philip Kaludercic <philipk <at> posteo.net> writes:
> Stefan Monnier <monnier <at> iro.umontreal.ca> 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?
> +
Yes, sorry will change that.
> ;; Author: rahguzar <rahguzar <at> zohomail.eu>
> ;; Maintainer: rahguzar <rahguzar <at> zohomail.eu>
> ;; 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!
Is the other definition in some package? I didn't see any filechooser
group is options for `customize-group` without this package loaded.
> "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
Will (if (> emacs-major-version 27) 'key 'string) be fine here?
> ;;;; 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?
This is a hack to get a default-directory I can use for a fake dired
buffer. I think it is enough to create such a directory once when the
package is loaded. I will try changing it so that the existence of this
directory is checked at the point of use and if not it is created.
> (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
Will do.
> (if transform
> cand
> (if (cdr (alist-get cand filechooser--filters nil nil #'equal))
Thanks,
Rahguzar
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.