Subject: bug#75956: Various improvements to sieve.el

From: Björn Bidar <[email-masked]>
Date: Sat, 08 Feb 2025 19:06:39 +0200

Eli Zaretskii <[email-masked]> writes:

>> Date: Fri, 31 Jan 2025 03:54:11 +0200
>> From:  Björn Bidar via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>>
>> I noticed it is not possible to specific the sieve-manage port when
>> calling `sieve-manage`. While fixing that I noticed various other issues.
>> - Missing documentation strings on many functions
>> - There is function to refresh the buffer but it is not bound
>>
>> The attached patches try to fix these issues.
>
> Thanks, please see some comments below.
>
>>  (defun sieve-manage (server &optional port)
>> -  (interactive "sServer: ")
>> +  "Open ManageSieve SERVER.
>> +Optionally with PORT.
>
> This is too cryptic.  It should be something like
>
>   Optional argument PORT specifies the port to use; it defaults to
>   `sieve-manage-default-port'.

Isn't that kinda redundant? Especially saying port twice doesn't help to
understand the purpose the variable is self explanatory. 

>> +When called in interactively prompt for SERVER:PORT
>                ^^              ^
> This "in" is redundant.  And a comma is missing after "interactively".
>
>> +if :PORT is not specified fallback to `sieve-manage-default-port'."
>                              ^^^^^^^^
> "fall back", as two words.
>
>>  (defun sieve-upload (&optional name)
>> +  "Upload to script NAME to currently opened server.
>              ^^
> That "to" should be removed.
>
>> +If NAME is nil detect name from script buffer name;
>                  ^
> Comma missing.  Also, "name" should be "NAME".  And the semicolon at
> the end should be a period.

Replaced the second NAME with it.

>> +If no open sieve buffer exists call `sieve-manage' first."
>                                  ^
> Comma missing.
>
>>  (defun sieve-upload-and-bury (&optional name)
>> +  "Upload script NAME and bury buffer."
>
> This should say "bury the current buffer", not just "bury buffer".
>
>>  ;;;###autoload
>>  (defun sieve-upload-and-kill (&optional name)
>> +  "Upload script NAME and kill buffer."
>
> Likewise.
>
>> >From 4c5a9c6267989d888b0b8aaa4a5b91508cd62d6e Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <[email-masked]>
>> Date: Fri, 31 Jan 2025 03:24:44 +0200
>> Subject: [PATCH 3/3] * lisp/net/sieve.el: Bind sieve-refresh-scriptlist to
>>  refresh buffer
>>
>> (sieve-manage-mode-map: Bind `sieve-refersh-scriptlist' to 'g' to refresh
>> buffer.
>> ---
>>  lisp/net/sieve.el | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el
>> index cd7053a7e65..98f1832d58d 100644
>> --- a/lisp/net/sieve.el
>> +++ b/lisp/net/sieve.el
>> @@ -111,6 +111,7 @@ sieve-manage-mode-map
>>    ;; various
>>    "?"      #'sieve-help
>>    "h"      #'sieve-help
>> +  "g"      #'sieve-refresh-scriptlist
>>    ;; activating
>>    "m"      #'sieve-activate
>>    "u"      #'sieve-deactivate
>> --
>
> This needs a NEWS entry about the new binding.

Done that while doing so I noticed that thghe info manual also needs
updating.
Updated that higlighted that the function updates the current sieve
buffer.

>From 7b2d6c3c4bfaa0611c6a23b452c0b40831c41696 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <[email-masked]>
Date: Fri, 31 Jan 2025 02:34:19 +0200
Subject: [PATCH 1/3] * lisp/net/sieve.el: Read port in sieve-manage when
 interactive

(sieve-manage): Optionally read port when called interactively.
Add previously missing documentation string.
---
 lisp/net/sieve.el | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el
index f9f036e1589..c67d8b34ce0 100644
--- a/lisp/net/sieve.el
+++ b/lisp/net/sieve.el
@@ -338,7 +338,14 @@ sieve-refresh-scriptlist
 
 ;;;###autoload
 (defun sieve-manage (server &optional port)
-  (interactive "sServer: ")
+  "Open ManageSieve SERVER.
+Optionally argument PORT specifies the port use;
+otherwise it defaults to `sieve-manage-default-port'.
+
+When called interactively, prompt for SERVER:PORT
+if :PORT is not specified fall back to `sieve-manage-default-port'."
+  (interactive
+   (split-string (read-string "Server: ") ":"))
   (switch-to-buffer (get-buffer-create sieve-buffer))
   (sieve-manage-mode)
   (sieve-setup-buffer server port)
-- 
2.45.2

>From a634a9bc7f01f15849706a377aae61d3c6921953 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <[email-masked]>
Date: Fri, 31 Jan 2025 02:53:34 +0200
Subject: [PATCH 2/3] * lisp/net/sieve.el: Add missing documentation strings

(sieve-activate, sieve-deactivate-all, sieve-remove)
(sieve-edit-script, sieve-refersh-scriptlist)
(sieve-upload, sieve-upload-and-burry)
(sieve-upload-and-kill): Add documentation strings
---
 lisp/net/sieve.el | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el
index c67d8b34ce0..b714e54b01a 100644
--- a/lisp/net/sieve.el
+++ b/lisp/net/sieve.el
@@ -161,6 +161,7 @@ sieve-bury-buffer
   (bury-buffer))
 
 (defun sieve-activate (&optional _pos)
+  "Activate script at point."
   (interactive)
   (let ((name (sieve-script-at-point)) err)
     (when (or (null name) (string-equal name sieve-new-script))
@@ -173,6 +174,7 @@ sieve-activate
       (message "Activating script %s...failed: %s" name (nth 2 err)))))
 
 (defun sieve-deactivate-all (&optional _pos)
+  "Deactivate all currently active scripts."
   (interactive)
   (message "Deactivating scripts...")
   (let (;; (name (sieve-script-at-point))
@@ -185,6 +187,7 @@ sieve-deactivate-all
 (defalias 'sieve-deactivate #'sieve-deactivate-all)
 
 (defun sieve-remove (&optional _pos)
+  "Remove script at point."
   (interactive)
   (let ((name (sieve-script-at-point)) err)
     (when (or (null name) (string-equal name sieve-new-script))
@@ -197,6 +200,7 @@ sieve-remove
     (message "Removing sieve script %s...done" name)))
 
 (defun sieve-edit-script (&optional _pos)
+  "Edit script at point."
   (interactive)
   (let ((name (sieve-script-at-point)))
     (unless name
@@ -310,6 +314,7 @@ sieve-open-server
     (sieve-manage-authenticate)))
 
 (defun sieve-refresh-scriptlist ()
+  "Refresh list of scripts found the currently opened server."
   (interactive)
   (with-current-buffer sieve-buffer
     (setq buffer-read-only nil)
@@ -355,6 +360,9 @@ sieve-manage
 
 ;;;###autoload
 (defun sieve-upload (&optional name)
+  "Upload script NAME to currently opened server.
+If NAME is nil, detect it from script buffer name.
+If no open sieve buffer exists, call `sieve-manage' first."
   (interactive)
   (when (or (get-buffer sieve-buffer)
             (save-current-buffer (call-interactively 'sieve-manage)))
@@ -373,12 +381,14 @@ sieve-upload
 
 ;;;###autoload
 (defun sieve-upload-and-bury (&optional name)
+  "Upload script NAME and bury the current buffer."
   (interactive)
   (sieve-upload name)
   (bury-buffer))
 
 ;;;###autoload
 (defun sieve-upload-and-kill (&optional name)
+  "Upload script NAME and kill the current buffer."
   (interactive)
   (sieve-upload name)
   (kill-buffer))
-- 
2.45.2

>From 90d6157a24906aac02cbacebebc2caeccfbf3a47 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <[email-masked]>
Date: Fri, 31 Jan 2025 03:24:44 +0200
Subject: [PATCH 3/3] Bind sieve-refresh-scriptlist to refresh buffer

* lisp/net/sieve.el (sieve-manage-mode-map):
Bind `sieve-refersh-scriptlist' to 'g' to refresh buffer.
(sieve-refresh-scriptlist): Highlight that the function updates
the current sieve buffer.

* etc/NEWS: Announce new keybinding

* doc/misc/siev.texi (Managing Sieve):
Document new keybinding and the existing `sieve-refresh-scriptlist`
function.
---
 doc/misc/sieve.texi | 6 ++++++
 etc/NEWS            | 6 ++++++
 lisp/net/sieve.el   | 4 +++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/doc/misc/sieve.texi b/doc/misc/sieve.texi
index 5d4b3b369d7..deadc991173 100644
--- a/doc/misc/sieve.texi
+++ b/doc/misc/sieve.texi
@@ -213,6 +213,12 @@ Managing Sieve
 @findex sieve-help
 Displays help in the minibuffer.
 
+@item g
+@kindex g
+@findex sieve-refresh-scriptlist
+Refresh list of scripts found on the currently opened server.
+Update contents of the current sieve buffer.
+
 @item Q
 @kindex Q
 @findex sieve-manage-quit
diff --git a/etc/NEWS b/etc/NEWS
index 2f04204ad94..fc40b07fa59 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -392,6 +392,12 @@ When called with a prefix argument, accepting, declining, or tentatively
 accepting an icalendar event will prompt for a comment to add to the
 response.
 
+** Sieve
++++
+*** New keybinding to refresh buffer in sieve-manage-mode
+'sieve-refresh-scriptlist' is now bound to 'g' to refresh the contents
+of the current sieve buffer.
+
 ** Button
 
 +++
diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el
index b714e54b01a..b55f571e07c 100644
--- a/lisp/net/sieve.el
+++ b/lisp/net/sieve.el
@@ -111,6 +111,7 @@ sieve-manage-mode-map
   ;; various
   "?"      #'sieve-help
   "h"      #'sieve-help
+  "g"      #'sieve-refresh-scriptlist
   ;; activating
   "m"      #'sieve-activate
   "u"      #'sieve-deactivate
@@ -314,7 +315,8 @@ sieve-open-server
     (sieve-manage-authenticate)))
 
 (defun sieve-refresh-scriptlist ()
-  "Refresh list of scripts found the currently opened server."
+  "Refresh list of scripts found the currently opened server.
+Update contents of the current sieve buffer."
   (interactive)
   (with-current-buffer sieve-buffer
     (setq buffer-read-only nil)
-- 
2.45.2