GNU bug report logs -
#59407
[PATCH] Add Colors to proced
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59407 in the body.
You can then email your comments to 59407 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sun, 20 Nov 2022 10:27:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Laurence Warne <laurencewarne <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Sun, 20 Nov 2022 10:27:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi, attached is a patch I've recently been working to do with colorizing
proced buffers, similar to htop.
In particular, the current Emacs process id is highlighted purple in both
the process id and parent process id columns, session group leaders have
their process ids underlined, larger memory sizes for rss and vsize are
highlighted in darker shades of orange, and the first word in the args
property (the executable) is highlighted in blue - I've attached a couple
of screenshots.
The way I'd recommend to try it out would be:
(require 'proced)
(setq-default proced-auto-update-flag t)
(setq-default proced-auto-update-interval 1)
(setq proced-enable-color-flag t)
And then M-x proced.
Thanks, Laurence
[Message part 2 (text/html, inline)]
[0001-Add-colors-to-proced.patch (text/x-patch, attachment)]
[proced-colours-1.png (image/png, attachment)]
[proced-colours-2.png (image/png, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sun, 20 Nov 2022 10:48:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 59407 <at> debbugs.gnu.org (full text, mbox):
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Sun, 20 Nov 2022 10:26:35 +0000
>
> Hi, attached is a patch I've recently been working to do with colorizing proced buffers, similar to htop.
>
> In particular, the current Emacs process id is highlighted purple in both the process id and parent process id
> columns, session group leaders have their process ids underlined, larger memory sizes for rss and vsize
> are highlighted in darker shades of orange, and the first word in the args property (the executable) is
> highlighted in blue - I've attached a couple of screenshots.
Thanks. Please be sure to test the new faces with the following Emacs
configurations:
. GUI frames with dark background
. GUI frames with light background
. TTY frames with dark and light backgrounds and with:
- 8 colors
- 16 colors
It is quite possible that some of the above combinations will not look well
with the colors you propose, and will need separate defaults.
A few more minor comments:
> * lisp/proced.el (proced-grammar-alist): update to use new format functions
> (proced-low-memory-usage-threshold): new custom variable to determine
> whether a value represents 'low' memory usage, used only in
> proced-format-memory for coloring
This is not our style of commit log messages. The description of each
change should begin with a capital letter and end with a period, i.e. be a
complete English sentence or several sentences.
> (proced-run-status-code): new face
> (proced-interruptible-sleep-status-code): new face
> (proced-uninterruptible-sleep-status-code): new face
> (proced-executable): new face
> (proced-memory-gb): new face
> (proced-memory-mb): new face
> (proced-memory-default): new face
> (proced-pid): new face
> (proced-ppid): new face
> (proced-pgrp): new face
> (proced-sess): new face
> (proced-cpu): new face
> (proced-mem): new face
> (proced-user): new face
> (proced-time-colon): new face
It is better to have a single list of new faces with one description, like
this:
(proced-run-status-code, proced-executable. proced-memory-default)
(proced-memory-mb, proced-pgrp): New faces.
> +(defcustom proced-enable-color-flag nil
> + "Non-nil means some process attributes will be rendered with color."
Please use "displayed", not "rendered".
> +(defcustom proced-low-memory-usage-threshold (* 1024 1024 100)
> + "The low memory usage (in bytes) upper bound.
This should probably be specified as percentage of total memory. Or maybe
there should be a separate defcustom for the percentage, and the condition
should use both. Just a single absolute threshold seems to cover only some
reasons for highlighting processes with large memory footprint.
> +(defcustom proced-medium-memory-usage-threshold (* 1024 1024 1024)
> + "The medium memory usage (in bytes) upper bound.
Likewise.
Finally, this changeset needs a suitable NEWS entry to announce it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sun, 20 Nov 2022 12:35:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 59407 <at> debbugs.gnu.org (full text, mbox):
Laurence Warne <laurencewarne <at> gmail.com> writes:
> Hi, attached is a patch I've recently been working to do with
> colorizing proced buffers, similar to htop.
>
> In particular, the current Emacs process id is highlighted purple in
> both the process id and parent process id columns, session group
> leaders have their process ids underlined, larger memory sizes for rss
> and vsize are highlighted in darker shades of orange, and the first
> word in the args property (the executable) is highlighted in blue -
> I've attached a couple of screenshots.
Thanks, but what exactly is the purpose of this change?
The more colors that need to be allocated, the slower Emacs becomes over
a wide-area network. In addition, every time a new color is used,
xfont_draw needs to be called again, generating more network traffic.
Emacs has already become quite slow over a network connection (though
this should have become significantly better in Emacs 29.)
Adding "eye candy" where it is not really necessary will only be a step
backwards.
> (require 'proced)
> (setq-default proced-auto-update-flag t)
> (setq-default proced-auto-update-interval 1)
> (setq proced-enable-color-flag t)
I guess if it is off by default, then I have no objections. But I
respectfully ask everyone to keep in mind the network impact of changes
they make to Emacs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sun, 20 Nov 2022 14:16:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 59407 <at> debbugs.gnu.org (full text, mbox):
Laurence Warne <laurencewarne <at> gmail.com> writes:
> Hi,
Hi Laurence,
> attached is a patch I've recently been working to do with
> colorizing proced buffers, similar to htop.
I haven't tried your patch, but I'm curious whether it works also for
proced buffers of remote systems.
> In particular, the current Emacs process id is highlighted purple in
> both the process id and parent process id columns,
This should happen only for proced running on the local system.
> Thanks, Laurence
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sun, 20 Nov 2022 14:41:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 59407 <at> debbugs.gnu.org (full text, mbox):
> Cc: 59407 <at> debbugs.gnu.org
> Date: Sun, 20 Nov 2022 20:33:59 +0800
> From: Po Lu via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>
> I guess if it is off by default, then I have no objections.
Yes, it is off by default, so will affect only users who turn it on.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Mon, 21 Nov 2022 09:09:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 59407 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thanks for all the feedback, I have a few queries.
> Thanks. Please be sure to test the new faces with the following Emacs
> configurations:
>
> . GUI frames with dark background
> . GUI frames with light background
> . TTY frames with dark and light backgrounds and with:
> - 8 colors
> - 16 colors
>
Do you know if there's an easy way I can test the faces on 8/16 colour
terminals?
> (proced-run-status-code, proced-executable. proced-memory-default)
> (proced-memory-mb, proced-pgrp): New faces.
>
Minor, but do you mean to add a closing paren at the end of each line? I
saw this format in the commit log:
(proced-run-status-code, proced-executable. proced-memory-default,
proced-memory-mb, proced-pgrp): New faces.
This should probably be specified as percentage of total memory. Or maybe
> there should be a separate defcustom for the percentage, and the condition
> should use both. Just a single absolute threshold seems to cover only some
> reasons for highlighting processes with large memory footprint.
>
I was thinking highlighting based on percentage memory would be more suited
to the "mem" process attribute (granted though this is not implemented).
Though in hindsight a global threshold may not make sense if you're
connecting to remote systems with varying amounts of RAM. Perhaps the two
thresholds could mark a percentage, say 10% and 50% of total memory?
I haven't tried your patch, but I'm curious whether it works also for
> proced buffers of remote systems.
>
Hi Michael, I did a quick check yesterday and it seemed to work as expected.
This should happen only for proced running on the local system.
>
Thanks, this should be fixed in the most recent patch.
I've attached a more up to date patch, with changes to faces (mainly adding
better defaults for light backgrounds), and a NEWS entry. Also attached
are images showing the current colour schemes on light backgrounds and
terminals.
Thanks, Laurence
[Message part 2 (text/html, inline)]
[proced-colors-light-1.png (image/png, attachment)]
[proced-colors-terminal-dark.png (image/png, attachment)]
[proced-terminal-light.png (image/png, attachment)]
[0001-Add-colors-to-proced.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Mon, 21 Nov 2022 10:33:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 59407 <at> debbugs.gnu.org (full text, mbox):
Laurence Warne <laurencewarne <at> gmail.com> writes:
> Hi Michael,
Hi Laurence,
> I did a quick check yesterday and it seemed to work as
> expected.
>
> This should happen only for proced running on the local system.
>
> Thanks, this should be fixed in the most recent patch.
Thanks!
> Thanks, Laurence
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Mon, 21 Nov 2022 14:29:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 59407 <at> debbugs.gnu.org (full text, mbox):
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Mon, 21 Nov 2022 09:07:59 +0000
> Cc: Po Lu <luangruo <at> yahoo.com>, 59407 <at> debbugs.gnu.org,
> Michael Albinus <michael.albinus <at> gmx.de>
>
> Thanks. Please be sure to test the new faces with the following Emacs
> configurations:
>
> . GUI frames with dark background
> . GUI frames with light background
> . TTY frames with dark and light backgrounds and with:
> - 8 colors
> - 16 colors
>
> Do you know if there's an easy way I can test the faces on 8/16 colour terminals?
For 8 colors, invoke Emacs with "-nw --color".
For 16 colors, try --color=16, and if it doesn't work, try configuring xterm
for 16 colors, then invoke Emacs with -nw.
> (proced-run-status-code, proced-executable. proced-memory-default)
> (proced-memory-mb, proced-pgrp): New faces.
>
> Minor, but do you mean to add a closing paren at the end of each line?
Yes. You can use the command "C-x 4 a" to format the log entries, it will
do this automatically if auto-fill is turned on.
> I saw this format in the commit log:
> (proced-run-status-code, proced-executable. proced-memory-default,
> proced-memory-mb, proced-pgrp): New faces.
This is wrong.
> This should probably be specified as percentage of total memory. Or maybe
> there should be a separate defcustom for the percentage, and the condition
> should use both. Just a single absolute threshold seems to cover only some
> reasons for highlighting processes with large memory footprint.
>
> I was thinking highlighting based on percentage memory would be more suited to the "mem" process
> attribute (granted though this is not implemented). Though in hindsight a global threshold may not make
> sense if you're connecting to remote systems with varying amounts of RAM. Perhaps the two thresholds
> could mark a percentage, say 10% and 50% of total memory?
Something like that, yes.
Thanks.
Severity set to 'wishlist' from 'normal'
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org
.
(Thu, 24 Nov 2022 18:28:05 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Fri, 25 Nov 2022 09:35:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 59407 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
I've attached a new patch, the changes this time are that the memory
thresholds now take a proportion rather than a fixed value, and some of the
face defaults have been improved to play nicer with 8 colour displays.
I've disabled the threshold highlighting for vsize since I don't really
think it makes sense: it represents more than just size in virtual memory +
size in RAM so I don't see a reasonable way to take it as a proportion
(though I don't think a fixed threshold makes sense either, so I've left it
unhighlighted). Also, memory-info doesn't appear to take into account
working on remote systems like list-system-processes does, so I've also
disabled the threshold highlighting for proced connected to remote
machines. Suggestions on this are welcome (:
I've attached a screenshot showing an 8 colour display, still figuring out
how I can get only 16 colours, I'll ping here again when I figure it out.
Thanks, Laurence
[Message part 2 (text/html, inline)]
[proced-8-color-light.png (image/png, attachment)]
[0001-Add-colors-to-proced.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Fri, 25 Nov 2022 11:31:01 GMT)
Full text and
rfc822 format available.
Message #34 received at 59407 <at> debbugs.gnu.org (full text, mbox):
Laurence Warne <laurencewarne <at> gmail.com> writes:
> Hi,
Hi Laurence,
> Also, memory-info doesn't
> appear to take into account working on remote systems like
> list-system-processes does, so I've also disabled the threshold
> highlighting for proced connected to remote machines. Suggestions on
> this are welcome (:
Indeed. Should we make memory-info aware of remote systems, like we have
done with list-system-processes and file-system-info? At least for
remote GNU/Linux systems, reading /proc/meminfo seems to be
easy. Proper commands for *BSD and Darwin systems shall also be applicable.
Eli?
> Thanks, Laurence
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Fri, 25 Nov 2022 15:08:02 GMT)
Full text and
rfc822 format available.
Message #37 received at 59407 <at> debbugs.gnu.org (full text, mbox):
> From: Michael Albinus <michael.albinus <at> gmx.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, luangruo <at> yahoo.com, 59407 <at> debbugs.gnu.org
> Date: Fri, 25 Nov 2022 12:30:33 +0100
>
> > Also, memory-info doesn't
> > appear to take into account working on remote systems like
> > list-system-processes does, so I've also disabled the threshold
> > highlighting for proced connected to remote machines. Suggestions on
> > this are welcome (:
>
> Indeed. Should we make memory-info aware of remote systems, like we have
> done with list-system-processes and file-system-info? At least for
> remote GNU/Linux systems, reading /proc/meminfo seems to be
> easy. Proper commands for *BSD and Darwin systems shall also be applicable.
>
> Eli?
I guess it would be nice, although not terribly important.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Fri, 25 Nov 2022 15:20:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 59407 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
Hi Eli,
>> Indeed. Should we make memory-info aware of remote systems, like we have
>> done with list-system-processes and file-system-info? At least for
>> remote GNU/Linux systems, reading /proc/meminfo seems to be
>> easy. Proper commands for *BSD and Darwin systems shall also be applicable.
>>
>> Eli?
>
> I guess it would be nice, although not terribly important.
But also not terribly hard to implement, I suppose. I'll give it a try,
and see how it goes.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sat, 26 Nov 2022 09:42:01 GMT)
Full text and
rfc822 format available.
Message #43 received at 59407 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> But also not terribly hard to implement, I suppose. I'll give it a try,
> and see how it goes.
Great, thanks Michael!
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sat, 26 Nov 2022 12:48:02 GMT)
Full text and
rfc822 format available.
Message #46 received at 59407 <at> debbugs.gnu.org (full text, mbox):
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Fri, 25 Nov 2022 09:34:09 +0000
> Cc: luangruo <at> yahoo.com, 59407 <at> debbugs.gnu.org, michael.albinus <at> gmx.de
>
> I've attached a new patch, the changes this time are that the memory thresholds now take a proportion
> rather than a fixed value, and some of the face defaults have been improved to play nicer with 8 colour
> displays.
Thanks, see some comments below.
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -514,6 +514,22 @@ option) and can be set to nil to disable Just-in-time Lock mode.
>
> * Changes in Emacs 29.1
>
> +---
> +** New user option `proced-enable-color-flag` to enable coloring of proced buffers
> +This option enables prompts some format functions to furnish their
> +respective process attributes with colors and effects in order to
> +make them easier to distinguish and highlight possible issues
> +(e.g. high memory usage), in a manner similar to htop.
> +
> +In particular, the current Emacs process id is highlighted purple in
> +both the process id and parent process id columns, session leaders
> +have their process ids underlined, larger memory sizes for rss are
> +highlighted in darker shades of orange, and the first word in the
> +args property (the executable) is highlighted in blue.
> +
> +Note this option is disabled by default and needs setting to a non-nil
> +value to take effect.
This is too long for a NEWS entry for such a minor feature. Please make it
shorter. Just saying that some fields of Proced display will be shown in
distinct colors, and mentioning the new defcustom to turn that on, should be
enough.
> +(defcustom proced-enable-color-flag nil
> + "Non-nil means some process attributes will be displayed with color."
Our style is to avoid passive tense whenever possible:
Non-nil means Proced should display some process attributes with color.
> +(defcustom proced-medium-memory-usage-threshold 0.5
> + "The medium memory usage (in bytes) upper bound.
It is better to avoid such constructs. Instead, say this:
The upper bound for medium memory usage, relative to total memory.
Note that I removed "in bytes", since this is not the units in which this is
measured.
> +When `proced-enable-color-flag' is non-nil, rss values denoting a proportion
> +of memory less than this value, but greater than
> +`proced-low-memory-usage-threshold' will be displayed using the
^
Comma missing there.
> +`proced-memory-medium-usage' face. rss values denoting a greater proportion
I think "rss" should be in all-caps, as "RSS". Same for "VSIZE".
> +(defface proced-interruptible-sleep-status-code
> + '((((class color)) (:foreground "DimGrey"))
Is this color visible well on both dark and light backgrounds?
> + (t (:italic t)))
> + "Face used for the interruptible sleep status code character \"S\"."
> + :version "29.1")
Please mention Proced in all the doc strings of these faces, to make it
clear they are only used by Proced.
> +(defface proced-emacs-pid
> + '((((class color) (min-colors 88)) (:foreground "purple"))
> + (((class color)) (:foreground "magenta")))
> + "Face for the pid of the current Emacs process."
^^^
Please use "process ID", not just its abbreviation.
> +(defface proced-pid
> + '((((class color) (min-colors 88)) (:foreground "#5085ef"))
> + (((class color)) (:foreground "blue")))
> + "Face for process ids."
"Face for process IDs", note the letter-case (here and elsewhere in the
patch).
> +(defface proced-cpu
> + '((((class color) (min-colors 88)) (:foreground "#6d5cc3" :bold t))
> + (t (:bold t)))
> + "Face for process cpu utilization."
"CPU", in caps.
Thanks for working on this.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Sun, 27 Nov 2022 16:05:01 GMT)
Full text and
rfc822 format available.
Message #49 received at 59407 <at> debbugs.gnu.org (full text, mbox):
Laurence Warne <laurencewarne <at> gmail.com> writes:
Hi Laurence,
>> But also not terribly hard to implement, I suppose. I'll give it a try,
>> and see how it goes.
>
> Great, thanks Michael!
I've pushed a respective change to master. On remote GNU/Linux or *BSD
systems, memory-info shall return the appropriate values now. On other
remote systems it returns nil.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Tue, 29 Nov 2022 14:04:01 GMT)
Full text and
rfc822 format available.
Message #52 received at 59407 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Eli,
Thanks for taking another look.
> Is this color visible well on both dark and light backgrounds?
I think it looks good on both, I've attached a couple of images to
hopefully highlight this (in the STAT column), let me know what you think.
Hopefully the attached patch addresses all of the rest of your comments.
> I've pushed a respective change to master. On remote GNU/Linux or *BSD
> systems, memory-info shall return the appropriate values now. On other
> remote systems it returns nil.
Nice, the new patch should take this into consideration.
Thanks, Laurence
[Message part 2 (text/html, inline)]
[proced-colours-1.png (image/png, attachment)]
[proced-colors-light-2.png (image/png, attachment)]
[0001-Add-colors-to-proced.patch (text/x-patch, attachment)]
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>
:
You have taken responsibility.
(Thu, 01 Dec 2022 18:19:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Laurence Warne <laurencewarne <at> gmail.com>
:
bug acknowledged by developer.
(Thu, 01 Dec 2022 18:19:02 GMT)
Full text and
rfc822 format available.
Message #57 received at 59407-done <at> debbugs.gnu.org (full text, mbox):
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Tue, 29 Nov 2022 14:02:52 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, luangruo <at> yahoo.com, 59407 <at> debbugs.gnu.org
>
> Nice, the new patch should take this into consideration.
Thanks, installed on the release branch, and closing the bug.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#59407
; Package
emacs
.
(Thu, 01 Dec 2022 21:15:02 GMT)
Full text and
rfc822 format available.
Message #60 received at 59407-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Great, thanks for your patience.
[Message part 2 (text/html, inline)]
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Fri, 30 Dec 2022 12:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 228 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.