GNU bug report logs - #78794
[PATCH] Pretiffy package-menu

Previous Next

To reply to this bug, email your comments to 78794 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Pretiffy package-menu
Date: Sat, 14 Jun 2025 20:07:27 -0600

[Message part 1 (text/plain, inline)]

Tags: patch


Hello, This patch adds a few visual improvements to `package-menu-mode`.

This add a new mode-line for package-menu which displays information
about: Total packages installed, Total packages from all the packages
archive, Total packages to upgrade and Total new packages available.

And highlight packages marked for installation or deletion.
I find this useful as the characters/tags can go unnoticed.

Here is a screenshot of the result:

[Captura desde 2025-06-14 20-05-30.png (image/png, inline)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: 78794 <at> debbugs.gnu.org
Subject: [PATCH] Pretiffy package-menu
Date: Sat, 14 Jun 2025 20:12:34 -0600

[0001-Pretiffy-package-menu-Bug-78794.patch (text/x-patch, attachment)]

[Message part 2 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78794 <at> debbugs.gnu.org
Subject: Re: bug#78794: [PATCH] Pretiffy package-menu
Date: Sun, 15 Jun 2025 08:36:20 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Date: Sat, 14 Jun 2025 20:12:34 -0600
> 
> +---
> +*** New user option 'package-menu-mode-line'.
> +If non-nil, package-menu will display additional information about Total
> +packages installed, Total packages from all the package archives, Total
> +packages to upgrade and Total new packages available.

Any reason not to have this by default, and drop the option?

> +(defface package-mark-install-line
> +  '((((class color) (min-colors 88) (background light))
> +     :background "darkolivegreen1" :extend t)
> +    (((class color) (min-colors 88) (background dark))
> +     :background "seagreen" :extend t)
> +    (((class color) (min-colors 8))
> +     :background "green" :foreground "white" :extend t)

If tty-color translation produces "green" from the two green shades
you define for 88+ color displays, then the last part is not needed.

> +    (t :inverse-video t :extend t))

Isn't the mode line shown in inverse video by default in monochrome
case?  If so, this is not needed, either.

> +(defface package-mark-delete-line
> +  '((((class color) (min-colors 88) (background light))
> +     :background "rosybrown1" :extend t)
> +    (((class color) (min-colors 88) (background dark))
> +     :background "indianred4" :extend t)
> +    (((class color) (min-colors 8))
> +     :background "red" :foreground "white" :extend t)
> +    (t :inverse-video t :extend t))
> +  "Face used for highlight the line where a package is marked to delete."
> +  :version "31.1")

Same here.

> +(define-fringe-bitmap
> +  'package-menu-install
> +  (vector
> +   #b000000
> +   #b001100
> +   #b001100
> +   #b111111
> +   #b111111
> +   #b001100
> +   #b001100
> +   #b000000)
> +  nil nil 'center)
> +
> +(define-fringe-bitmap
> +  'package-menu-delete
> +  (vector
> +   #b000000
> +   #b000000
> +   #b000000
> +   #b111111
> +   #b111111
> +   #b000000
> +   #b000000
> +   #b000000)
> +  nil nil 'center)

Both colors _and_ fringe markers?  Isn't than too much?

> +;; Mode-line
> +(defcustom package-menu-mode-line t
> +  "If non-nil, package-menu will display additional information in mode-line.
> +package-menu mode-line information includes: Total packages installed,
> +Total packages from all the packages archive, Total packages to upgrade
> +and Total new packages available."

Capitalization is wrong here: the last sentence should start with a
capital letter, whereas the "Total something" parts should not be
capitalized.

> +              ;; XXX: Add a efficient way for set these variables (?)

What about this part?

Thanks.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78794 <at> debbugs.gnu.org
Subject: Re: bug#78794: [PATCH] Pretiffy package-menu
Date: Sun, 15 Jun 2025 13:02:06 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Date: Sat, 14 Jun 2025 20:12:34 -0600
>> 
>> +---
>> +*** New user option 'package-menu-mode-line'.
>> +If non-nil, package-menu will display additional information about Total
>> +packages installed, Total packages from all the package archives, Total
>> +packages to upgrade and Total new packages available.
>
> Any reason not to have this by default, and drop the option?

This is enabled by default (I forgot to include it in the NEWS entry).
The reason why i decided make it an option is because some user
may find it annoying.

>> +(defface package-mark-install-line
>> +  '((((class color) (min-colors 88) (background light))
>> +     :background "darkolivegreen1" :extend t)
>> +    (((class color) (min-colors 88) (background dark))
>> +     :background "seagreen" :extend t)
>> +    (((class color) (min-colors 8))
>> +     :background "green" :foreground "white" :extend t)
>
> If tty-color translation produces "green" from the two green shades
> you define for 88+ color displays, then the last part is not needed.

Fine.

>> +    (t :inverse-video t :extend t))
>
> Isn't the mode line shown in inverse video by default in monochrome
> case?  If so, this is not needed, either.

These faces are not for the mode-line, are for the package-menu buffer,
specifically for highlight the line where a package was marked for
install/delete.

>> +(define-fringe-bitmap
>> +  'package-menu-install
>> +  (vector
>> +   #b000000
>> +   #b001100
>> +   #b001100
>> +   #b111111
>> +   #b111111
>> +   #b001100
>> +   #b001100
>> +   #b000000)
>> +  nil nil 'center)
>> +
>> +(define-fringe-bitmap
>> +  'package-menu-delete
>> +  (vector
>> +   #b000000
>> +   #b000000
>> +   #b000000
>> +   #b111111
>> +   #b111111
>> +   #b000000
>> +   #b000000
>> +   #b000000)
>> +  nil nil 'center)
>
> Both colors _and_ fringe markers?  Isn't than too much?

Well, I don't know what others say about this, the fringes were
originally intended as an alternative to the "I" and "D" marks.

Anyways I've added a better alternative to fringes.

>> +;; Mode-line
>> +(defcustom package-menu-mode-line t
>> +  "If non-nil, package-menu will display additional information in mode-line.
>> +package-menu mode-line information includes: Total packages installed,
>> +Total packages from all the packages archive, Total packages to upgrade
>> +and Total new packages available."
>
> Capitalization is wrong here: the last sentence should start with a
> capital letter, whereas the "Total something" parts should not be
> capitalized.

Thanks, fixed.

>> +              ;; XXX: Add a efficient way for set these variables (?)
>
> What about this part?

Oh, that was temporal, I was supposed to remove it before making the
patch.

[0001-Pretiffy-package-menu-Bug-78794.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>,
 Philip Kaludercic <philipk <at> posteo.net>
Cc: 78794 <at> debbugs.gnu.org
Subject: Re: bug#78794: [PATCH] Pretiffy package-menu
Date: Mon, 16 Jun 2025 09:29:27 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 78794 <at> debbugs.gnu.org
> Date: Sun, 15 Jun 2025 13:02:06 -0600
> 
> >> +*** New user option 'package-menu-mode-line'.
> >> +If non-nil, package-menu will display additional information about Total
> >> +packages installed, Total packages from all the package archives, Total
> >> +packages to upgrade and Total new packages available.
> >
> > Any reason not to have this by default, and drop the option?
> 
> This is enabled by default (I forgot to include it in the NEWS entry).
> The reason why i decided make it an option is because some user
> may find it annoying.

In what ways could it be annoying?

Philip, do you see any reason to make this customizable with a boolean
variable, and not via the usual mode-line customizations?  Will many
users want to remove this new information?

> >> +(defface package-mark-install-line
> >> +  '((((class color) (min-colors 88) (background light))
> >> +     :background "darkolivegreen1" :extend t)
> >> +    (((class color) (min-colors 88) (background dark))
> >> +     :background "seagreen" :extend t)
> >> +    (((class color) (min-colors 8))
> >> +     :background "green" :foreground "white" :extend t)
> >
> > If tty-color translation produces "green" from the two green shades
> > you define for 88+ color displays, then the last part is not needed.
> 
> Fine.
> 
> >> +    (t :inverse-video t :extend t))
> >
> > Isn't the mode line shown in inverse video by default in monochrome
> > case?  If so, this is not needed, either.
> 
> These faces are not for the mode-line, are for the package-menu buffer,
> specifically for highlight the line where a package was marked for
> install/delete.

Then I think inverse-video is too radical.  I'd suggest to use bold
(if supported) or underline (if supported), with inverse-video being
the last resort.

> +(defface package-mark-install-line
> +  '((((class color) (min-colors 88) (background light))
> +     :background "darkolivegreen1" :extend t)
> +    (((class color) (min-colors 88) (background dark))
> +     :background "seagreen" :extend t)
> +    (t :inverse-video t :extend t))

This will cause any display with fewer than 88 colors use the
inverse-video alternative.  I suggest to remove the min-colors
requirement, since it isn't needed.

> +(defface package-mark-delete-line
> +  '((((class color) (min-colors 88) (background light))
> +     :background "rosybrown1" :extend t)
> +    (((class color) (min-colors 88) (background dark))
> +     :background "indianred4" :extend t)
> +    (t :inverse-video t :extend t))

Likewise.

> +(defcustom package-menu-marks-indicators
> +  '((install . "I")
> +    (delete . "D"))
> +  "Alist indicators to indicate a package is marked to install or delete.
> +The value of each list must be in the formt: '(KIND . MARK) where KIND
> +is the kind mark performed (`install' or `delete') in package-menu and
> +MARK a string to use for mark the packages.  This currently support
> +marks for install and delete."

This doc string has several grammatical issues, and also lacks some
important information.  I suggest to rephrase as follows:

   "Indicators for packages to be installed or deleted.
 The value is an alist whose elements have the form (KIND . MARK),
 where KIND is the operation to perform, either `install' or `delete',
 and MARK is a string to indicate that the operation is pending for
 the package.  The MARK string should satisfy the requirements of the
 TAG argument of `tabulated-list-put-tag', which see.  It is displayed
 in the padding area of the package's line.

 Currently, only indicators for installing or deleting a package are
 supported."

> +  :type '(list (cons symbol string)
> +               (cons symbol string))

Can we come up with a meaningful :tag for the value?

> +(defun package-menu--overlay-line (face)
> +  "Highlight whole line with face FACE."
> +  (let ((ov (make-overlay (line-beginning-position)
> +                          (1+ (line-end-position)))))
> +    (overlay-put ov 'pkg-menu-ov t)
> +    (overlay-put ov 'evaporate t)
> +    (overlay-put ov 'face face)))

Does package-menu use other faces in overlays, and if so, should we
consider giving this overlay a non-default priority?

> +                  (total-help "Total packages of all package archives")
> +                  (installed-help "Total packages installed")
> +                  (upgrade-help "Total packages to upgrade")
> +                  (new-help "Total new packages available"))

Please use "Total number of SOMETHING" in these help strings.
Otherwise, it is not immediately clear what "Total" refers to.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78794 <at> debbugs.gnu.org,
 Elijah Gabe Pérez <eg642616 <at> gmail.com>
Subject: Re: bug#78794: [PATCH] Pretiffy package-menu
Date: Mon, 16 Jun 2025 12:47:10 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Cc: 78794 <at> debbugs.gnu.org
>> Date: Sun, 15 Jun 2025 13:02:06 -0600
>> 
>> >> +*** New user option 'package-menu-mode-line'.
>> >> +If non-nil, package-menu will display additional information about Total
>> >> +packages installed, Total packages from all the package archives, Total
>> >> +packages to upgrade and Total new packages available.
>> >
>> > Any reason not to have this by default, and drop the option?
>> 
>> This is enabled by default (I forgot to include it in the NEWS entry).
>> The reason why i decided make it an option is because some user
>> may find it annoying.
>
> In what ways could it be annoying?
>
> Philip, do you see any reason to make this customizable with a boolean
> variable, and not via the usual mode-line customizations?  Will many
> users want to remove this new information?

I agree that it doesn't seem like something that should be too annoying.
From the description, it reminds me of Flymake, that also displays the
number of warnings and errors by default.  I haven't heard frequent
complaints about that.

>> >> +(defface package-mark-install-line
>> >> +  '((((class color) (min-colors 88) (background light))
>> >> +     :background "darkolivegreen1" :extend t)
>> >> +    (((class color) (min-colors 88) (background dark))
>> >> +     :background "seagreen" :extend t)
>> >> +    (((class color) (min-colors 8))
>> >> +     :background "green" :foreground "white" :extend t)
>> >
>> > If tty-color translation produces "green" from the two green shades
>> > you define for 88+ color displays, then the last part is not needed.
>> 
>> Fine.
>> 
>> >> +    (t :inverse-video t :extend t))
>> >
>> > Isn't the mode line shown in inverse video by default in monochrome
>> > case?  If so, this is not needed, either.
>> 
>> These faces are not for the mode-line, are for the package-menu buffer,
>> specifically for highlight the line where a package was marked for
>> install/delete.
>
> Then I think inverse-video is too radical.  I'd suggest to use bold
> (if supported) or underline (if supported), with inverse-video being
> the last resort.

I would use the `highlight' face.  We could also add a semantic
"selection".

>> +(defface package-mark-install-line
>> +  '((((class color) (min-colors 88) (background light))
>> +     :background "darkolivegreen1" :extend t)
>> +    (((class color) (min-colors 88) (background dark))
>> +     :background "seagreen" :extend t)
>> +    (t :inverse-video t :extend t))
>
> This will cause any display with fewer than 88 colors use the
> inverse-video alternative.  I suggest to remove the min-colors
> requirement, since it isn't needed.
>
>> +(defface package-mark-delete-line
>> +  '((((class color) (min-colors 88) (background light))
>> +     :background "rosybrown1" :extend t)
>> +    (((class color) (min-colors 88) (background dark))
>> +     :background "indianred4" :extend t)
>> +    (t :inverse-video t :extend t))
>
> Likewise.
>
>> +(defcustom package-menu-marks-indicators
>> +  '((install . "I")
>> +    (delete . "D"))
>> +  "Alist indicators to indicate a package is marked to install or delete.
>> +The value of each list must be in the formt: '(KIND . MARK) where KIND
>> +is the kind mark performed (`install' or `delete') in package-menu and
>> +MARK a string to use for mark the packages.  This currently support
>> +marks for install and delete."

What is the reason we would want to customise this in the first place?

>
> This doc string has several grammatical issues, and also lacks some
> important information.  I suggest to rephrase as follows:
>
>    "Indicators for packages to be installed or deleted.
>  The value is an alist whose elements have the form (KIND . MARK),
>  where KIND is the operation to perform, either `install' or `delete',
>  and MARK is a string to indicate that the operation is pending for
>  the package.  The MARK string should satisfy the requirements of the
>  TAG argument of `tabulated-list-put-tag', which see.  It is displayed
>  in the padding area of the package's line.
>
>  Currently, only indicators for installing or deleting a package are
>  supported."
>
>> +  :type '(list (cons symbol string)
>> +               (cons symbol string))
>
> Can we come up with a meaningful :tag for the value?

(I haven't check it, but...) is this the type of
`package-menu-marks-indicators'?  If so, we shouldn't document the user
option as an Alist.

>> +(defun package-menu--overlay-line (face)
>> +  "Highlight whole line with face FACE."
>> +  (let ((ov (make-overlay (line-beginning-position)
>> +                          (1+ (line-end-position)))))
>> +    (overlay-put ov 'pkg-menu-ov t)
>> +    (overlay-put ov 'evaporate t)
>> +    (overlay-put ov 'face face)))
>
> Does package-menu use other faces in overlays, and if so, should we
> consider giving this overlay a non-default priority?

From a quick search, we don't appear to use any overlays in package.el.

>> +                  (total-help "Total packages of all package archives")
>> +                  (installed-help "Total packages installed")
>> +                  (upgrade-help "Total packages to upgrade")
>> +                  (new-help "Total new packages available"))
>
> Please use "Total number of SOMETHING" in these help strings.
> Otherwise, it is not immediately clear what "Total" refers to.

1+

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78794 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#78794: [PATCH] Pretiffy package-menu
Date: Mon, 16 Jun 2025 13:12:53 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> This is enabled by default (I forgot to include it in the NEWS entry).
>> The reason why i decided make it an option is because some user
>> may find it annoying.
>
> In what ways could it be annoying?

Fine, I've removed the option.

>> These faces are not for the mode-line, are for the package-menu buffer,
>> specifically for highlight the line where a package was marked for
>> install/delete.
>
> Then I think inverse-video is too radical.  I'd suggest to use bold
> (if supported) or underline (if supported), with inverse-video being
> the last resort.

I've changed it to use `highlight` face instead, following Philip
suggestion.

>> +(defcustom package-menu-marks-indicators
>> +  '((install . "I")
>> +    (delete . "D"))
>> +  "Alist indicators to indicate a package is marked to install or delete.
>> +The value of each list must be in the formt: '(KIND . MARK) where KIND
>> +is the kind mark performed (`install' or `delete') in package-menu and
>> +MARK a string to use for mark the packages.  This currently support
>> +marks for install and delete."
>
> This doc string has several grammatical issues, and also lacks some
> important information.  I suggest to rephrase as follows:
>
>    "Indicators for packages to be installed or deleted.
>  The value is an alist whose elements have the form (KIND . MARK),
>  where KIND is the operation to perform, either `install' or `delete',
>  and MARK is a string to indicate that the operation is pending for
>  the package.  The MARK string should satisfy the requirements of the
>  TAG argument of `tabulated-list-put-tag', which see.  It is displayed
>  in the padding area of the package's line.
>
>  Currently, only indicators for installing or deleting a package are
>  supported."

Thanks, it sounds better.

>> +(defun package-menu--overlay-line (face)
>> +  "Highlight whole line with face FACE."
>> +  (let ((ov (make-overlay (line-beginning-position)
>> +                          (1+ (line-end-position)))))
>> +    (overlay-put ov 'pkg-menu-ov t)
>> +    (overlay-put ov 'evaporate t)
>> +    (overlay-put ov 'face face)))
>
> Does package-menu use other faces in overlays, and if so, should we
> consider giving this overlay a non-default priority?

AFAIK, there are no other overlays in package-menu.

[0001-Pretiffy-package-menu-Bug-78794.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 78794 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#78794: [PATCH] Pretiffy package-menu
Date: Mon, 16 Jun 2025 13:52:32 -0600

Philip Kaludercic <philipk <at> posteo.net> writes:

>>> +(defcustom package-menu-marks-indicators
>>> +  '((install . "I")
>>> +    (delete . "D"))
>>> +  "Alist indicators to indicate a package is marked to install or delete.
>>> +The value of each list must be in the formt: '(KIND . MARK) where KIND
>>> +is the kind mark performed (`install' or `delete') in package-menu and
>>> +MARK a string to use for mark the packages.  This currently support
>>> +marks for install and delete."
>
> What is the reason we would want to customise this in the first place?

IMO, the I and D characters looks somewhat ugly to me, making them
customizable allows to change them to an Unicode/Emoji icon (or any
other character), making them more recognizable according to the user's
preferences (e.g. 🗑️/♻️ and ✅).

>> This doc string has several grammatical issues, and also lacks some
>> important information.  I suggest to rephrase as follows:
>>
>>    "Indicators for packages to be installed or deleted.
>>  The value is an alist whose elements have the form (KIND . MARK),
>>  where KIND is the operation to perform, either `install' or `delete',
>>  and MARK is a string to indicate that the operation is pending for
>>  the package.  The MARK string should satisfy the requirements of the
>>  TAG argument of `tabulated-list-put-tag', which see.  It is displayed
>>  in the padding area of the package's line.
>>
>>  Currently, only indicators for installing or deleting a package are
>>  supported."
>>
>>> +  :type '(list (cons symbol string)
>>> +               (cons symbol string))
>>
>> Can we come up with a meaningful :tag for the value?
>
> (I haven't check it, but...) is this the type of
> `package-menu-marks-indicators'?  If so, we shouldn't document the user
> option as an Alist.

But what other name would be better?

Honestly I decided to make this option a list instead of making a
separated option for each mark, this is in case there are plans to add
more options/marks in the future.

-- 
                                          - E.G via GNU Emacs and Org.

Previous Next

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