GNU bug report logs - #59842
[PATCH] Make proced-update Preserve Refinements

Previous Next

Package: emacs;

Reported by: Laurence Warne <laurencewarne <at> gmail.com>

Date: Mon, 5 Dec 2022 20:27:01 UTC

Severity: wishlist

Tags: patch

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

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59842 in the body.
You can then email your comments to 59842 AT debbugs.gnu.org in the normal way.

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#59842; Package emacs. (Mon, 05 Dec 2022 20: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. (Mon, 05 Dec 2022 20:27:02 GMT) Full text and rfc822 format available.

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

From: Laurence Warne <laurencewarne <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Make proced-update Preserve Refinements
Date: Mon, 5 Dec 2022 20:26:26 +0000
[Message part 1 (text/plain, inline)]
Hi,

Currently proced-update will clear any active refinements in proced-buffers
(see proced-refine for information on refinements), which can be annoying
when proced-auto-update-flag is non-nil as this will result in you only
being able to see the refinement for a few seconds before the buffer
updates and you're back to all processes.  To reproduce this:

(require 'proced)
(setq-default proced-auto-update-flag t)
(setq-default proced-auto-update-interval 1)

M-x proced, then create a new refinement by <ENTER> on the PID of any
process.  You should see your refinement vanish after the next update.

The patch fixes aims to fix this by introducing a new buffer local variable
"proced-refinements" which stores information about the current
refinements, and is used by proced-update to further refine
proced-process-alist in the case it is non-nil.

proced-revert will get rid of any existing refinements (bound to "g"), so
the existing behaviour of refinements with proced-auto-update-flag set to
nil should stay the same.

Thanks, Laurence
[Message part 2 (text/html, inline)]
[0001-Make-proced-update-preserve-refinements.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59842; Package emacs. (Tue, 06 Dec 2022 17:50:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Laurence Warne <laurencewarne <at> gmail.com>
Cc: 59842 <at> debbugs.gnu.org
Subject: Re: bug#59842: [PATCH] Make proced-update Preserve Refinements
Date: Tue, 06 Dec 2022 19:49:06 +0200
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Mon, 5 Dec 2022 20:26:26 +0000
> 
> The patch fixes aims to fix this by introducing a new buffer local variable "proced-refinements" which stores
> information about the current refinements, and is used by proced-update to further refine
> proced-process-alist in the case it is non-nil.

Thanks.  Unfortunately, we don't have a test suite for proced.el, so
non-trivial changes to it always ruin the risk of producing regressions.
How to test this, and how did you test it?

> * lisp/proced.el (proced-refinements): New buffer local variable
> which tracks the current refinements.
> (proced-refine): Set proced-refinements variable and defer setting of
> proced-process-alist to proced-update.
> (proced-update): Take into account proced-refinements when setting
> proced-process-alist.
> (proced-revert): Set proced-refinements to nil prior to calling proced-update.

Please quote symbols in log entries 'like this'.  (Do not quote symbols
inside parentheses, only in the descriptive text.)

> +  (pcase-dolist (`(,refiner ,pid ,key ,grammar) proced-refinements)
> +    ;; It's possible the process has exited since the refinement was made
> +    (when (assq pid proced-process-alist)

What happens if that process did exit?  Shouldn't we reset
proced-refinements to nil?

>  (defun proced-revert (&rest _args)
>    "Reevaluate the process listing based on the currently running processes.
> -Preserves point and marks."
> +Preserves point and marks, but not refinements."

Please add a cross-reference to proced-refine here, so that the reader who
doesn't know what a "refinement" is could find out.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59842; Package emacs. (Thu, 08 Dec 2022 19:07:02 GMT) Full text and rfc822 format available.

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

From: Laurence Warne <laurencewarne <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59842 <at> debbugs.gnu.org
Subject: Re: bug#59842: [PATCH] Make proced-update Preserve Refinements
Date: Thu, 8 Dec 2022 19:06:35 +0000
[Message part 1 (text/plain, inline)]
> Thanks.  Unfortunately, we don't have a test suite for proced.el, so
> non-trivial changes to it always ruin the risk of producing regressions.
> How to test this, and how did you test it?

If it's helpful, I've attached a (seperate) patch containing a test suite
(or at least the start of) for proced.el (though some parts are somewhat
awkward - mainly testing the proced buffer contains strings we would expect
- of course comments on the approach welcome), the last test there:
'proced-refine-with-update-test' fails without the original patch.  I
didn't want to conflate the original patch with it, I can open a new report
with it if you prefer.

> What happens if that process did exit?  Shouldn't we reset
> proced-refinements to nil?

This could lead to bad behaviour if multiple refinements are active in a
buffer, for example if I refine by CPU usage of process A (which makes
proced only show processes with usage >= process A's) and then again by the
CPU usage of process B, if process A exits, our second refinement is still
valid (given process B is still running) eventhough the first isn't.  We
could remove the refinement from the list, but it wouldn't change the
behaviour.

Thanks, Laurence
[Message part 2 (text/html, inline)]
[0001-Make-proced-update-preserve-refinements.patch (text/x-patch, attachment)]
[0001-Add-tests-for-proced.patch (text/x-patch, attachment)]

Severity set to 'wishlist' from 'normal' Request was from Stefan Kangas <stefankangas <at> gmail.com> to control <at> debbugs.gnu.org. (Tue, 13 Dec 2022 01:21:04 GMT) Full text and rfc822 format available.

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Wed, 14 Dec 2022 14:56:02 GMT) Full text and rfc822 format available.

Notification sent to Laurence Warne <laurencewarne <at> gmail.com>:
bug acknowledged by developer. (Wed, 14 Dec 2022 14:56:02 GMT) Full text and rfc822 format available.

Message #18 received at 59842-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Laurence Warne <laurencewarne <at> gmail.com>
Cc: 59842-done <at> debbugs.gnu.org
Subject: Re: bug#59842: [PATCH] Make proced-update Preserve Refinements
Date: Wed, 14 Dec 2022 16:55:05 +0200
> From: Laurence Warne <laurencewarne <at> gmail.com>
> Date: Thu, 8 Dec 2022 19:06:35 +0000
> Cc: 59842 <at> debbugs.gnu.org
> 
> If it's helpful, I've attached a (seperate) patch containing a test suite (or at least the start of) for proced.el
> (though some parts are somewhat awkward - mainly testing the proced buffer contains strings we would
> expect - of course comments on the approach welcome), the last test there: 'proced-refine-with-update-test'
> fails without the original patch.  I didn't want to conflate the original patch with it, I can open a new report with it
> if you prefer.

I installed both on the master branch, and modified the tests so that
they could be run on more systems.

I'm therefore closing this bug.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 12 Jan 2023 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 76 days ago.

Previous Next


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