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

From: Rahguzar <[email-masked]>
Date: Mon, 05 Feb 2024 22:45:45 +0100

Hi Philip,

Philip Kaludercic <[email-masked]> writes:

>> Is the other definition in some package? I didn't see any filechooser
>> group is options for `customize-group` without this package loaded.
>
> I believe there was a duplicate definition a few lines above.
>

Thanks a lot for catching this. Should be fixed now.

>>> @@ -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?
>
> I think 'key-sequence would be the more adequate fallback.

Thanks, changed now.

>>>  ;;;; 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.
>
> Right, that sounds more robust.  I don't know all the edge cases in
> which a temporary directory can be removed, especially over different
> operating systems, so I'd recommend not relying on it, especially over
> longer periods.
>

I have changed this too.

>>>  (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.
>
> Thanks.
>

I have referred to info node. Is is possible to refer to a specific
paragraph in the info manual? Ideally I would like to point to the text
about group function there.

>>>    (if transform
>>>        cand
>>>      (if (cdr (alist-get cand filechooser--filters nil nil #'equal))
>>
>> Thanks,
>> Rahguzar

Thanks a lot,
Rahguzar