bug#36707: [PATCH] Signal user-error on duplicate package refresh

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Stefan Kangas]

In the "*Packages*" buffer (M-x package-list-packages), if I hit "r" many times
it will try to initiate more refreshes even if the first one has not finished
yet.  This is unnecessary and potentially confusing.

This patch makes it check if there is already an refresh in progress and signal
a user-error if there is one.

(I'll send the patch in a separate message once this is assigned a bug
number.)

Thanks,
Stefan Kangas

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Stefan Kangas]

[-- Attachment #1: Type: text/plain, Size: 166 bytes --]

Stefan Kangas <stefan@marxist.se> writes:
> (I'll send the patch in a separate message once this is assigned a bug
> number.)

Patch attached.

Thanks,
Stefan Kangas

[-- Attachment #2: 0001-Signal-a-user-error-on-duplicate-package-refresh.patch --]
[-- Type: application/octet-stream, Size: 1641 bytes --]

From a7ba44b7eb78067a84e09800edda75e955e626db Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Wed, 17 Jul 2019 19:55:08 +0200
Subject: [PATCH] Signal a user-error on duplicate package refresh

* lisp/emacs-lisp/package.el (package-menu-refresh): Signal a
user-error if there is already a refresh running in the background.
Doc fix.  (Bug#36707)
---
 lisp/emacs-lisp/package.el | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 9a350aadac..d46207a328 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -3042,12 +3042,15 @@ package-menu--old-archive-contents
   "`package-archive-contents' before the latest refresh.")
 
 (defun package-menu-refresh ()
-  "Download the Emacs Lisp package archive.
-This fetches the contents of each archive specified in
-`package-archives', and then refreshes the package menu."
+  "In Package Menu, download the Emacs Lisp package archive.
+Fetch the contents of each archive specified in
+`package-archives', and then refresh the package menu.  Signal a
+user-error if there is already a refresh running asynchronously."
   (interactive)
   (unless (derived-mode-p 'package-menu-mode)
     (user-error "The current buffer is not a Package Menu"))
+  (when (and package-menu-async package--downloads-in-progress)
+    (user-error "Package refresh is already in progress, please wait..."))
   (setq package-menu--old-archive-contents package-archive-contents)
   (setq package-menu--new-package-list nil)
   (package-refresh-contents package-menu-async))
-- 
2.22.0

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> In the "*Packages*" buffer (M-x package-list-packages), if I hit "r" many times
> it will try to initiate more refreshes even if the first one has not finished
> yet.  This is unnecessary and potentially confusing.
>
> This patch makes it check if there is already an refresh in progress and signal
> a user-error if there is one.

Would it not be possible/better to discard the existing refresh and
start a new one, instead of signalling an error?  This would help
e.g. when a refresh hangs for whatever reason.

Thanks,

-- 
Basil

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Noam Postavsky]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> In the "*Packages*" buffer (M-x package-list-packages), if I hit "r" many times
>> it will try to initiate more refreshes even if the first one has not finished
>> yet.  This is unnecessary and potentially confusing.
>>
>> This patch makes it check if there is already an refresh in progress and signal
>> a user-error if there is one.
>
> Would it not be possible/better to discard the existing refresh and
> start a new one, instead of signalling an error?  This would help
> e.g. when a refresh hangs for whatever reason.

Not sure if it's better, supposing the user is hitting "r" repeatedly,
that just interrupts the pending refreshes, preventing any progress.  I
would expect hanging to be cancelled by a timeout anyway.

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Stefan Kangas]

Noam Postavsky <npostavs@gmail.com> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
> > Stefan Kangas <stefan@marxist.se> writes:
> >
> >> In the "*Packages*" buffer (M-x package-list-packages), if I hit "r" many times
> >> it will try to initiate more refreshes even if the first one has not finished
> >> yet.  This is unnecessary and potentially confusing.
> >>
> >> This patch makes it check if there is already an refresh in progress and signal
> >> a user-error if there is one.
> >
> > Would it not be possible/better to discard the existing refresh and
> > start a new one, instead of signalling an error?  This would help
> > e.g. when a refresh hangs for whatever reason.
>
> Not sure if it's better, supposing the user is hitting "r" repeatedly,
> that just interrupts the pending refreshes, preventing any progress.  I
> would expect hanging to be cancelled by a timeout anyway.

I agree with Noam that a timeout would make more sense.  Not sure if
it already exists or if that's a separate issue.

Thanks,
Stefan Kangas

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Stefan Kangas]

Stefan Kangas <stefan@marxist.se> writes:

> Noam Postavsky <npostavs@gmail.com> writes:
>
> > "Basil L. Contovounesios" <contovob@tcd.ie> writes:
> >
> > > Stefan Kangas <stefan@marxist.se> writes:
> > >
> > >> In the "*Packages*" buffer (M-x package-list-packages), if I hit "r" many times
> > >> it will try to initiate more refreshes even if the first one has not finished
> > >> yet.  This is unnecessary and potentially confusing.
> > >>
> > >> This patch makes it check if there is already an refresh in progress and signal
> > >> a user-error if there is one.
> > >
> > > Would it not be possible/better to discard the existing refresh and
> > > start a new one, instead of signalling an error?  This would help
> > > e.g. when a refresh hangs for whatever reason.
> >
> > Not sure if it's better, supposing the user is hitting "r" repeatedly,
> > that just interrupts the pending refreshes, preventing any progress.  I
> > would expect hanging to be cancelled by a timeout anyway.
>
> I agree with Noam that a timeout would make more sense.  Not sure if
> it already exists or if that's a separate issue.

Are there any other objections to the patch?  Otherwise, I'll go ahead
and push it in a few days.

Thanks,
Stefan Kangas

bug#36707: [PATCH] Signal user-error on duplicate package refresh

[Stefan Kangas]

close 36707 27.1
quit

Stefan Kangas <stefan@marxist.se> writes:

> Are there any other objections to the patch?  Otherwise, I'll go ahead
> and push it in a few days.

I've now pushed this as commit a6d87ea045.  Closing the report.

Thanks,
Stefan Kangas