Re: Request to add Package to GNU ELPA

[Philip Kaludercic <philipk@posteo.net>]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +;; Package-Requires: ((emacs "26.1") (compat "29.1.0.0"))
>
> [ I'm curious which definitions of `compat` this uses.  ]

Those should be

- file-name-concat
- length>
- with-suppressed-warnings
- named-let
- length=
- ensure-list
- macroexp-file-name
- dlet

Do you think it is worth mentioning them?

>> @@ -1208,10 +1214,13 @@ package-buffer-info
>>         :kind 'single
>>         :url website
>>         :keywords keywords
>> -       :maintainer
>> +       :maintainer (lm-maintainer)
>>         ;; For backward compatibility, use a single string if there's only
>>         ;; one maintainer (the most common case).
>> -       (let ((maints (lm-maintainers))) (if (cdr maints) maints (car maints)))
>> +       (if (fboundp 'lm-maintainers)
>> +           (let ((maints (lm-maintainers)))
>> +             (if (cdr maints) maints (car maints)))
>> +         (lm-maintainer))
>>         :authors (lm-authors)))))
>
> Hmm... isn't the `(lm-maintainer)` right after `:maintainer` erroneous?

Yes, that was a typo.

> [ BTW, I think Jonas has another change pending for this code.  ]
>
>>        (when news
>> -        (insert "\n" (make-separator-line) "\n"
>> -                (propertize "* News" 'face 'package-help-section-name)
>> +        (newline)
>> +        (when (fboundp 'make-separator-line)
>> +          (insert (make-separator-line))
>> +          (newline))
>> +        (insert (propertize "* News" 'face 'package-help-section-name)
>>                  "\n\n")
>>          (insert-file-contents news))
>
> Why not just replace (make-separator-line) with
> (if (fboundp 'make-separator-line) (make-separator-line) "")?

I believe what I had in mind was to avoid formatting issues when
`make-separator-line' was not available.  I'll check again.

> `newline` is a command and I think it's preferable not to use it
> from ELisp.

Oops, right.

>> @@ -4571,8 +4581,11 @@ package-report-bug
>>        (dolist (ent (get (cdr group) 'custom-group))
>>          (when (and (custom-variable-p (car ent))
>>                     (boundp (car ent))
>> -                   (not (eq (custom--standard-value (car ent))
>> -                            (default-toplevel-value (car ent))))
>> +                   (not (eq
>> +                         ;; We are not using `custom--standard-value'
>> +                         ;; to retain compatibility for Emacs 27.
>> +                         (eval (car (get (car ent) 'standard-value)) t)
>> +                         (default-toplevel-value (car ent))))
>
> I'd use
>
>     (if (fboundp 'custom--standard-value) ;; Emacs≥27
>         (custom--standard-value (car ent))
>       (eval (car (get (car ent) 'standard-value)) t))
>
> so the code "speaks for itself".

Good point.

-- 
Philip Kaludercic