GNU bug report logs - #76978
31.0.50; Archive information not displayed for installed packages in *Packages* buffer

Previous Next

Full log

Message #20 received at 76978 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: david <davidimagid <at> gmail.com>
Cc: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>,
 76978 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: [PATCH] package.el: Add built-in version check and active
 package marking
Date: Tue, 18 Mar 2025 14:34:18 -0400

> 1. Built-in version check: The package menu now compares the built-in
> version of a package with the available version.  If the available
> version is newer, the status is set to "available"; otherwise, it is
> marked as "built-in".  This helps users identify when updates are
> available for built-in packages.

[ I can't remember enough of the details about this
  part of the UI to have an opinion on that.  ]

> 2. Activated packages in "Other versions": Packages listed in the
> "Other versions" section are now marked with ", activated" if they are
> currently active in the Emacs session.  This makes it clear which
> versions are in use.

Nice!

The commit message lacks the ChangeLog-style itemized changes.
See further comments below.

Also, I can't see your name in the list of people who signed the
copyright paperwork.  Are you already in the process of doing that?

> @@ -1230,7 +1229,7 @@ package-tar-file-info
>                         ((filename (tar-header-name (car tar-parse-info))))
>                       (let ((dirname (file-name-directory filename)))
>                         ;; The first file can be in a subdir: look for the top.
> -                       (if dirname (loop (directory-file-name dirname))
> +                       (if dirname (cl-loop (directory-file-name dirname))
>                           (file-name-as-directory filename)))))
>           (desc-file (package--description-file dir-name))
>           (tar-desc (tar-get-file-descriptor (concat dir-name desc-file))))

This doesn't look right: `loop` is a locally defined function.

>                                (from (or (package-desc-archive opkg)
> -                                        (if (stringp dir) "installed" dir))))
> +                                        (if (package-desc-dir opkg)
> +                                            "installed" "built-in")))

Any reason you dropped the `stringp` test?

> +                              ;; Check if the package is currently activated and
> +                              ;; matches the described version.
> +                              (activated (and (member (package-desc-name opkg)
> +                                                      package-activated-list)
> +                                              (equal ov (package-desc-version desc)))))

`desc` is not necessarily the package that is currently activated, so
I don't understand the `equal` test here.

Also, AFAICT, this goes past the 80 columns limit, please try and change
the layout to stay within 80 columns.

> -                         (if (not ov) (format "%s" from)
> -                           (format "%s (%s)"
> -                                   (make-text-button (package-version-join ov) nil
> -                                                     'font-lock-face 'link
> -                                                     'follow-link t
> -                                                     'action
> -                                                     (lambda (_button)
> -                                                       (describe-package opkg)))
> -                                   from))))
> +                         (format "%s (%s%s)"
> +                                 (if ov
> +                                     (make-text-button (package-version-join ov) nil
> +                                                       'font-lock-face 'link
> +                                                       'follow-link t
> +                                                       'action
> +                                                       (lambda (_button)
> +                                                         (describe-package opkg)))
> +                                   from)
> +                                 from
> +                                 ;; Append ", activated" if the package is currently
> +                                 ;; activated.
> +                                 (if activated ", activated" ""))))

If `ov` is nil, we'll get FROM displayed twice.  🙁


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.