bug#25764: 26.0.50; Make some usability improvements to sieve-script management

["Basil L. Contovounesios" <contovob@tcd.ie>]

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

I don't use sieve scripts, but I noticed a minor nit in your patch:

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> @@ -215,6 +214,7 @@ sieve-edit-script
>      (sieve-mode)
>      (setq sieve-buffer-script-name name)
>      (goto-char (point-min))
> +    (set-buffer-modified-p nil)
>      (message
>       (substitute-command-keys
>        "Press \\[sieve-upload] to upload script to server."))))

[...]

> @@ -350,11 +350,13 @@ sieve-upload
>        (with-current-buffer (get-buffer sieve-buffer)
>  	(setq err (sieve-manage-putscript
>                     (or name sieve-buffer-script-name (buffer-name))
> -                   script sieve-manage-buffer))
> -	(if (sieve-manage-ok-p err)
> -	    (message (substitute-command-keys
> -		      "Sieve upload done.  Use \\[sieve-manage] to manage scripts."))
> -	  (message "Sieve upload failed: %s" (nth 2 err)))))))
> +                   script sieve-manage-buffer)))
> +      (if (sieve-manage-ok-p err)
> +          (progn
> +            (message (substitute-command-keys
> +                      "Sieve upload done.  Use \\[sieve-manage] to manage scripts."))
> +            (set-buffer-modified-p nil))
> +        (message "Sieve upload failed: %s" (nth 2 err))))))

In both hunks, 'message' is given an arbitrary string as its first
argument.  Any objections to the following cleanup of sieve.el text
formatting code?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Tidy-up-sieve.el-text-formatting.patch --]
[-- Type: text/x-diff, Size: 4323 bytes --]

From 77e1def846770526f50fceca41e269ec9be5080b Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Mon, 8 Jul 2019 21:05:14 +0100
Subject: [PATCH] Tidy up sieve.el text formatting

* lisp/net/sieve.el (sieve-edit-script, sieve-upload): Do not pass
arbitrary string as first argument to 'message' (bug#25764).
(sieve-help): Split long string across multiple lines.
(sieve-refresh-scriptlist): Use ngettext.  Fix grammar.
---
 lisp/net/sieve.el | 50 +++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el
index 55fea160f6..3337998bed 100644
--- a/lisp/net/sieve.el
+++ b/lisp/net/sieve.el
@@ -215,9 +215,8 @@ sieve-edit-script
     (setq sieve-buffer-script-name name)
     (goto-char (point-min))
     (set-buffer-modified-p nil)
-    (message
-     (substitute-command-keys
-      "Press \\[sieve-upload] to upload script to server."))))
+    (message "Press %s to upload script to server."
+             (substitute-command-keys "\\[sieve-upload]"))))
 
 (defmacro sieve-change-region (&rest body)
   "Turns off sieve-region before executing BODY, then re-enables it after.
@@ -256,8 +255,10 @@ sieve-help
   (if (eq last-command 'sieve-help)
       ;; would need minor-mode for log-edit-mode
       (describe-function 'sieve-mode)
-    (message "%s" (substitute-command-keys
-	      "`\\[sieve-edit-script]':edit `\\[sieve-activate]':activate `\\[sieve-deactivate]':deactivate `\\[sieve-remove]':remove `\\[sieve-manage-quit]':quit"))))
+    (message "%s" (substitute-command-keys "\
+`\\[sieve-edit-script]':edit `\\[sieve-activate]':activate \
+`\\[sieve-deactivate]':deactivate `\\[sieve-remove]':remove \
+`\\[sieve-manage-quit]':quit"))))
 
 ;; Create buffer:
 
@@ -312,20 +313,20 @@ sieve-refresh-scriptlist
     (delete-region (or sieve-buffer-header-end (point-max)) (point-max))
     (goto-char (point-max))
     ;; get list of script names and print them
-    (let ((scripts (sieve-manage-listscripts sieve-manage-buffer)))
-      (if (null scripts)
-	  (insert
-           (substitute-command-keys
-            (format
-             "No scripts on server, press \\[sieve-edit-script] on %s to create a new script.\n"
-             sieve-new-script)))
-	(insert
-         (substitute-command-keys
-          (format (concat "%d script%s on server, press \\[sieve-edit-script] on a script "
-                          "name edits it, or\npress \\[sieve-edit-script] on %s to create "
-                          "a new script.\n") (length scripts)
-                          (if (eq (length scripts) 1) "" "s")
-                          sieve-new-script))))
+    (let* ((scripts (sieve-manage-listscripts sieve-manage-buffer))
+           (count (length scripts))
+           (keys (substitute-command-keys "\\[sieve-edit-script]")))
+      (insert
+       (if (null scripts)
+           (format
+            "No scripts on server, press %s on %s to create a new script.\n"
+            keys sieve-new-script)
+         (format (concat (ngettext "%d script on server"
+                                   "%d scripts on server"
+                                   count)
+                         ", press %s on a script name to edit it, or"
+                         "\npress %s on %s to create a new script.\n")
+                 count keys keys sieve-new-script)))
       (save-excursion
 	(sieve-insert-scripts (list sieve-new-script))
 	(sieve-insert-scripts scripts)))
@@ -354,12 +355,11 @@ sieve-upload
 	(setq err (sieve-manage-putscript
                    (or name sieve-buffer-script-name script-name)
                    script sieve-manage-buffer))
-	(if (sieve-manage-ok-p err)
-            (progn
-	      (message (substitute-command-keys
-		        "Sieve upload done.  Use \\[sieve-manage] to manage scripts."))
-              (set-buffer-modified-p nil))
-	  (message "Sieve upload failed: %s" (nth 2 err)))))))
+        (if (not (sieve-manage-ok-p err))
+            (message "Sieve upload failed: %s" (nth 2 err))
+          (message "Sieve upload done.  Use %s to manage scripts."
+                   (substitute-command-keys "\\[sieve-manage]"))
+          (set-buffer-modified-p nil))))))
 
 ;;;###autoload
 (defun sieve-upload-and-bury (&optional name)
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 20 bytes --]


Thanks,

-- 
Basil