GNU bug report logs - #59407
[PATCH] Add Colors to proced

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Laurence Warne <laurencewarne <at> gmail.com>
Cc: 59407 <at> debbugs.gnu.org
Subject: Re: bug#59407: [PATCH] Add Colors to proced
Date: Sun, 20 Nov 2022 12:48:03 +0200

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

Previous Next

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