GNU bug report logs - #25764
26.0.50; Make some usability improvements to sieve-script management

Previous Next

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

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

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.50; Make some usability improvements to sieve-script management
Date: Thu, 16 Feb 2017 13:13:41 -0800

[Message part 1 (text/plain, inline)]

The attached patch makes some usability improvements to the sieve script
management files: exposing more user commands in menus and help, and
using the buffer-modified mark to give the user a better sense of what's
going on.

[0001-Small-improvements-to-sieve-script-usability.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

In GNU Emacs 26.0.50.26 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.7)
 of 2017-02-14 built on clem
Repository revision: 71b90192dab8de9825904faaabbaf9548d3db2ab
Windowing system distributor 'The X.Org Foundation', version 11.0.11901000

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Tue, 25 Jun 2019 01:03:27 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> The attached patch makes some usability improvements to the sieve script
> management files: exposing more user commands in menus and help, and
> using the buffer-modified mark to give the user a better sense of what's
> going on.

I think the patch looks good; please apply.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 25764-done <at> debbugs.gnu.org, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Mon, 08 Jul 2019 10:55:22 -0700

On 06/25/19 01:03 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> The attached patch makes some usability improvements to the sieve script
>> management files: exposing more user commands in menus and help, and
>> using the buffer-modified mark to give the user a better sense of what's
>> going on.
>
> I think the patch looks good; please apply.

Done, thanks!

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Mon, 08 Jul 2019 21:42:51 +0100

[Message part 1 (text/plain, inline)]

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

Eric Abrahamsen <eric <at> 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?

[0001-Tidy-up-sieve.el-text-formatting.patch (text/x-diff, inline)]

From 77e1def846770526f50fceca41e269ec9be5080b Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob <at> 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

[Message part 3 (text/plain, inline)]

Thanks,

-- 
Basil

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Eric Abrahamsen <eric <at> ericabrahamsen.net>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Tue, 09 Jul 2019 00:09:49 +0200

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

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

I think that looks correct (but I don't use Sieve, either).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Mon, 08 Jul 2019 16:51:58 -0700

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

> I don't use sieve scripts, but I noticed a minor nit in your patch:
>
> Eric Abrahamsen <eric <at> 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?

I do use sieve! But I didn't write this code. Your proposed chances seem
fine to me, though I'm wondering about the use of `ngettext' -- I saw
some arguments on these lists recently about international/localization,
and wasn't paying too much attention, but didn't see a clear resolution.
Is it the consensus that we're supposed to be using `ngettext' where possible?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Tue, 09 Jul 2019 01:55:40 +0200

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> I do use sieve! But I didn't write this code. Your proposed chances seem
> fine to me, though I'm wondering about the use of `ngettext' -- I saw
> some arguments on these lists recently about international/localization,
> and wasn't paying too much attention, but didn't see a clear resolution.
> Is it the consensus that we're supposed to be using `ngettext' where possible?

Oh, I didn't notice that part.  No, I don't think there's any such
consensus...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Mon, 08 Jul 2019 17:12:12 -0700

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> I do use sieve! But I didn't write this code. Your proposed chances seem
>> fine to me, though I'm wondering about the use of `ngettext' -- I saw
>> some arguments on these lists recently about international/localization,
>> and wasn't paying too much attention, but didn't see a clear resolution.
>> Is it the consensus that we're supposed to be using `ngettext' where possible?
>
> Oh, I didn't notice that part.  No, I don't think there's any such
> consensus...

There are 23 uses of `ngettext' in the sources right now, which says to
me: "there's no consensus, but it's also not a problem".

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eric Abrahamsen <eric <at> ericabrahamsen.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Tue, 09 Jul 2019 01:50:38 +0100

Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>
>>> I do use sieve! But I didn't write this code.

I know, just thought I may as well fix these trivial instances upon
seeing your patch.

>>> Your proposed chances seem fine to me, though I'm wondering about
>>> the use of `ngettext' -- I saw some arguments on these lists
>>> recently about international/localization, and wasn't paying too
>>> much attention, but didn't see a clear resolution.  Is it the
>>> consensus that we're supposed to be using `ngettext' where possible?
>>
>> Oh, I didn't notice that part.  No, I don't think there's any such
>> consensus...
>
> There are 23 uses of `ngettext' in the sources right now, which says to
> me: "there's no consensus, but it's also not a problem".

That's more or less my understanding as well:

[45c525a685]: Fig grammar of count-lines-page
  2019-06-09 17:00:05 -0700
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=45c525a685041c274a26c70caed4088726a790de

[7bba702e1a]: Use ngettext instead of dired-plural-s in Dired (bug#35287)
  2019-05-05 22:43:27 +0300
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7bba702e1ad1ed343618e44cf5bbc2a1f079be0c

I wasn't involved in the discussion, but IIRC l10n (i.e. using a no-op
ngettext for now) is welcome and encouraged, it's just that we don't
have an agreed upon or concrete implementation yet.  I think it's
definitely better to mark the text as localisable now, while we're in
the area, than leave the previous string formatting trick for future
archaeologists to rediscover.  So I've now pushed my patch:

[7b70efb740]: Tidy up sieve.el text formatting
  2019-07-09 01:47:52 +0100
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7b70efb740a04517e2764b0986c6e92305c1dd3e

Thanks for the review and sorry for hijacking this report.

-- 
Basil

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

From: Eric Abrahamsen <eric <at> ericabrahamsen.net>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 25764 <at> debbugs.gnu.org
Subject: Re: bug#25764: 26.0.50;
 Make some usability improvements to sieve-script management
Date: Mon, 08 Jul 2019 18:06:33 -0700

On 07/09/19 01:50 AM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>>
>>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>>
>>>> I do use sieve! But I didn't write this code.
>
> I know, just thought I may as well fix these trivial instances upon
> seeing your patch.
>
>>>> Your proposed chances seem fine to me, though I'm wondering about
>>>> the use of `ngettext' -- I saw some arguments on these lists
>>>> recently about international/localization, and wasn't paying too
>>>> much attention, but didn't see a clear resolution.  Is it the
>>>> consensus that we're supposed to be using `ngettext' where possible?
>>>
>>> Oh, I didn't notice that part.  No, I don't think there's any such
>>> consensus...
>>
>> There are 23 uses of `ngettext' in the sources right now, which says to
>> me: "there's no consensus, but it's also not a problem".
>
> That's more or less my understanding as well:
>
> [45c525a685]: Fig grammar of count-lines-page
>   2019-06-09 17:00:05 -0700
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=45c525a685041c274a26c70caed4088726a790de
>
> [7bba702e1a]: Use ngettext instead of dired-plural-s in Dired (bug#35287)
>   2019-05-05 22:43:27 +0300
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7bba702e1ad1ed343618e44cf5bbc2a1f079be0c
>
> I wasn't involved in the discussion, but IIRC l10n (i.e. using a no-op
> ngettext for now) is welcome and encouraged, it's just that we don't
> have an agreed upon or concrete implementation yet.  I think it's
> definitely better to mark the text as localisable now, while we're in
> the area, than leave the previous string formatting trick for future
> archaeologists to rediscover.  So I've now pushed my patch:

Okay, fair enough...

Previous Next

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