GNU bug report logs - #75956
Various improvements to sieve.el

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 75956 in the body.
You can then email your comments to 75956 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: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Various improvements to sieve.el
Date: Fri, 31 Jan 2025 03:54:11 +0200

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

Tags: patch


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.


In GNU Emacs 31.0.50 (build 1, x86_64-suse-linux-gnu, GTK+ Version
3.24.43, cairo version 1.18.2)
Repository revision: 2efffbe7731ba979540df851819af0ef15b94c62
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101015
System Description: openSUSE Tumbleweed

Configured using:
 'configure --disable-build-details --without-pop --with-mailutils
 --without-hesiod --with-gameuser=:games --with-kerberos
 --with-kerberos5 --with-file-notification=inotify --with-modules
 --enable-autodepend --enable-link-time-optimization --prefix=/usr
 --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share
 --localstatedir=/var --sharedstatedir=/var/lib
 --libexecdir=/usr/libexec --with-file-notification=yes
 --libdir=/usr/lib64 --with-native-compilation=aot
 --enable-locallisppath=/usr/share/emacs/31.0.50/site-lisp:/usr/share/emacs/site-lisp
 --with-x --with-xim --with-sound --with-xpm --with-jpeg --with-tiff
 --with-gif --with-png --with-rsvg --with-dbus --with-xft --without-gpm
 --with-tree-sitter --with-x-toolkit=gtk --without-pgtk
 --with-toolkit-scroll-bars --x-includes=/usr/include
 --x-libraries=/usr/lib64 --with-libotf --with-m17n-flt --with-cairo
 --build=x86_64-suse-linux --with-dumping=pdumper
 build_alias=x86_64-suse-linux 'CC=sccache cc' 'CFLAGS=-O2 -Wall
 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong
 -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection
 -Werror=return-type -flto=auto -march=znver3 -mmmx -mpopcnt -msse
 -msse2 -msse3 -mssse3 -msse4.1 -msse4.2 -mavx -mavx2 -msse4a -mno-fma4
 -mno-xop -mfma -mbmi -mbmi2 -maes -mpclmul -mno-gfni -mvpclmulqdq
 -mno-3dnow -madx -mabm -mno-cldemote -mclflushopt -mclwb -mclzero
 -mcx16 -mno-enqcmd -mf16c -mfsgsbase -mfxsr -mno-hle -msahf -mno-lwp
 -mlzcnt -mmovbe -mno-movdir64b -mno-movdiri -mmwaitx -mno-pconfig -mpku
 -mprfchw -mno-ptwrite -mrdpid -mrdrnd -mrdseed -mno-rtm -mno-serialize
 -mno-sgx -msha -mshstk -mno-tbm -mno-tsxldtrk -mvaes -mno-waitpkg
 -mwbnoinvd -mxsave -mxsavec -mxsaveopt -mxsaves -mno-amx-tile
 -mno-amx-int8 -mno-amx-bf16 -mno-uintr -mno-hreset -mno-kl -mno-widekl
 -mno-avxvnni -mno-avxifma -mno-avxvnniint8 -mno-avxneconvert
 -mno-cmpccxadd -mno-amx-fp16 -mno-prefetchi -mno-raoint
 -mno-amx-complex --param l1-cache-size=32 --param l1-cache-line-size=64
 --param l2-cache-size=512 -mtune=znver3 -fno-optimize-sibling-calls -O2
 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong
 -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection
 -Werror=return-type -flto=auto -g -D_GNU_SOURCE
 -DGDK_DISABLE_DEPRECATION_WARNINGS -DGLIB_DISABLE_DEPRECATION_WARNINGS
 -pipe -Wno-pointer-sign -Wno-unused-variable -Wno-unused-label
 -DPDMP_BASE='\''"emacs-gtk"'\''' LDFLAGS=-Wl,-O2 'CXX=sccache c++'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

[0001-lisp-net-sieve.el-Read-port-in-sieve-manage-when-int.patch (text/patch, attachment)]

[0002-lisp-net-sieve.el-Add-missing-documentation-strings.patch (text/x-patch, attachment)]

[0003-lisp-net-sieve.el-Bind-sieve-refresh-scriptlist-to-r.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Sat, 08 Feb 2025 11:56:18 +0200

> 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" <bug-gnu-emacs <at> gnu.org>
> 
> 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'.

> +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.

> +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?= <bjorn.bidar <at> thaodan.de>
> 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.

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Sat, 08 Feb 2025 19:06:39 +0200

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

Eli Zaretskii <eliz <at> gnu.org> 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" <bug-gnu-emacs <at> gnu.org>
>>
>> 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?= <bjorn.bidar <at> thaodan.de>
>> 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.

[0001-lisp-net-sieve.el-Read-port-in-sieve-manage-when-int.patch (text/x-patch, attachment)]

[0002-lisp-net-sieve.el-Add-missing-documentation-strings.patch (text/x-patch, attachment)]

[0003-Bind-sieve-refresh-scriptlist-to-refresh-buffer.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>, 
 Eli Zaretskii <eliz <at> gnu.org>
Cc: 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Mon, 10 Feb 2025 23:59:57 -0800

Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> Eli Zaretskii <eliz <at> gnu.org> 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" <bug-gnu-emacs <at> gnu.org>
>>>
>>> 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.

Some additional comments from me.

>>>  (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.

It's not redundant, no.

> From 7b2d6c3c4bfaa0611c6a23b452c0b40831c41696 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <bjorn.bidar <at> thaodan.de>
> 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.

Please add the bug number here and to the other patches, like this:

    Add missing documentation string.  (Bug#75956)

> ---
>  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: ") ":"))

How are users that see a prompt for "Server: " supposed to know that
they are to enter a part also?  What will happen if the user doesn't do
that?

I recommend prompting first for server, then for port.  Also, how about
providing some defaults (does "localhost" and some standard port make
sense)?

The rest of your patches LGTM.

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Tue, 11 Feb 2025 17:44:24 +0200

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> 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" <bug-gnu-emacs <at> gnu.org>
>>>>
>>>> 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.
>
> Some additional comments from me.
>
>>>>  (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.
>
> It's not redundant, no.
>
>> From 7b2d6c3c4bfaa0611c6a23b452c0b40831c41696 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <bjorn.bidar <at> thaodan.de>
>> 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.
>
> Please add the bug number here and to the other patches, like this:
>
>     Add missing documentation string.  (Bug#75956)
>
>> ---
>>  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: ") ":"))
>
> How are users that see a prompt for "Server: " supposed to know that
> they are to enter a part also?  What will happen if the user doesn't do
> that?

As per the documentation string if the port isn't included the default
port is used which is sieve.

> I recommend prompting first for server, then for port.  Also, how about
> providing some defaults (does "localhost" and some standard port make
> sense)?

Specifying the port inside url is part of the standard why of specifying
the port (standard as is in general).

Asking for the port in the second step would also change the default
behavior.
A compromise would be to mention in the prompt that the port can be
specified in the prompt.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Tue, 11 Feb 2025 08:25:15 -0800

Björn Bidar <bjorn.bidar <at> thaodan.de> writes:

> A compromise would be to mention in the prompt that the port can be
> specified in the prompt.

OK, then let's do that.

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Tue, 11 Feb 2025 22:02:34 +0200

Stefan Kangas <stefankangas <at> gmail.com> writes:

>> A compromise would be to mention in the prompt that the port can be
>> specified in the prompt.
>
> OK, then let's do that.

What did you have in mind? For example for man pages the optional arguments
it is custom to put optional arguments in [].
Going with that the prompt could look like that: 'Server[:port] : '.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Tue, 11 Feb 2025 12:09:16 -0800

Björn Bidar <bjorn.bidar <at> thaodan.de> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>>> A compromise would be to mention in the prompt that the port can be
>>> specified in the prompt.
>>
>> OK, then let's do that.
>
> What did you have in mind? For example for man pages the optional arguments
> it is custom to put optional arguments in [].
> Going with that the prompt could look like that: 'Server[:port] : '.

Yeah, we have to be brief, which makes this hard.  How about this:

    Server (optionally followed by `:port'):

Eli, any better ideas?

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Tue, 11 Feb 2025 23:58:09 +0200

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

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Björn Bidar <bjorn.bidar <at> thaodan.de> writes:
>
>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>>> A compromise would be to mention in the prompt that the port can be
>>>> specified in the prompt.
>>>
>>> OK, then let's do that.
>>
>> What did you have in mind? For example for man pages the optional arguments
>> it is custom to put optional arguments in [].
>> Going with that the prompt could look like that: 'Server[:port] : '.
>
> Yeah, we have to be brief, which makes this hard.  How about this:
>
>     Server (optionally followed by `:port'):

OK changed the prompt to look like this. Regarding the bug reference
again.
When the patch touches multiple files should the reference go to the
first change only?

Anyway here the updated patches.

[0001-lisp-net-sieve.el-Read-port-in-sieve-manage-when-int.patch (text/x-patch, attachment)]

[0002-lisp-net-sieve.el-Add-missing-documentation-strings.patch (text/x-patch, attachment)]

[0003-Bind-sieve-refresh-scriptlist-to-refresh-buffer.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: bjorn.bidar <at> thaodan.de, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Wed, 12 Feb 2025 14:23:27 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Tue, 11 Feb 2025 12:09:16 -0800
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
> 
> Yeah, we have to be brief, which makes this hard.  How about this:
> 
>     Server (optionally followed by `:port'):
> 
> Eli, any better ideas?

That's not bad.  Another possibility:

  Server and port (SERVER[:PORT]):

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75956 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Wed, 12 Feb 2025 20:12:53 +0200

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

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Tue, 11 Feb 2025 12:09:16 -0800
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
>> 
>> Yeah, we have to be brief, which makes this hard.  How about this:
>> 
>>     Server (optionally followed by `:port'):
>> 
>> Eli, any better ideas?
>
> That's not bad.  Another possibility:
>
>   Server and port (SERVER[:PORT]):

Which one is preferred? Personally I'm fine with either. I already
updated the patch with Stefan's suggestion.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>, 
 Eli Zaretskii <eliz <at> gnu.org>
Cc: 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Wed, 12 Feb 2025 10:47:31 -0800

Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Stefan Kangas <stefankangas <at> gmail.com>
>>> Date: Tue, 11 Feb 2025 12:09:16 -0800
>>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
>>>
>>> Yeah, we have to be brief, which makes this hard.  How about this:
>>>
>>>     Server (optionally followed by `:port'):
>>>
>>> Eli, any better ideas?
>>
>> That's not bad.  Another possibility:
>>
>>   Server and port (SERVER[:PORT]):
>
> Which one is preferred? Personally I'm fine with either. I already
> updated the patch with Stefan's suggestion.

I think we can go with Eli's suggestion here.  Users that want to
understand things better should read the docstring.

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Wed, 12 Feb 2025 21:23:10 +0200

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

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> From: Stefan Kangas <stefankangas <at> gmail.com>
>>>> Date: Tue, 11 Feb 2025 12:09:16 -0800
>>>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
>>>>
>>>> Yeah, we have to be brief, which makes this hard.  How about this:
>>>>
>>>>     Server (optionally followed by `:port'):
>>>>
>>>> Eli, any better ideas?
>>>
>>> That's not bad.  Another possibility:
>>>
>>>   Server and port (SERVER[:PORT]):
>>
>> Which one is preferred? Personally I'm fine with either. I already
>> updated the patch with Stefan's suggestion.
>
> I think we can go with Eli's suggestion here.  Users that want to
> understand things better should read the docstring.

Ok there you go:

[0001-lisp-net-sieve.el-Read-port-in-sieve-manage-when-int.patch (text/x-patch, attachment)]

[0002-lisp-net-sieve.el-Add-missing-documentation-strings.patch (text/x-patch, attachment)]

[0003-Bind-sieve-refresh-scriptlist-to-refresh-buffer.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956-done <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Thu, 13 Feb 2025 01:34:09 -0800

Version: 31.1

Björn Bidar <bjorn.bidar <at> thaodan.de> writes:

> From 653a0196fd32c2fe0262beeb50e7fb74a0345d13 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <bjorn.bidar <at> thaodan.de>
> 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. (Bug#75956)

Thanks, I installed your patches on master with some minor touchups
(commits 07d1fdb1ea2, c5bfaf1ae3e, and 6967587daef).  Please take a look
at the changes I did for reference.

I'm consequently closing this bug report.

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956-done <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Thu, 13 Feb 2025 20:19:35 +0200

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Version: 31.1
>
> Björn Bidar <bjorn.bidar <at> thaodan.de> writes:
>
>> From 653a0196fd32c2fe0262beeb50e7fb74a0345d13 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Bj=C3=B6rn=20Bidar?= <bjorn.bidar <at> thaodan.de>
>> 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. (Bug#75956)
>
> Thanks, I installed your patches on master with some minor touchups
> (commits 07d1fdb1ea2, c5bfaf1ae3e, and 6967587daef).  Please take a look
> at the changes I did for reference.

The changes look good. For the future now I know that to put in the
message message, e.g. not including the news file change in the commit
message.

The change "Make sieve-manage prompt also for port number" is missing
the file name. That's ok?

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75956 <at> debbugs.gnu.org
Subject: Re: bug#75956: Various improvements to sieve.el
Date: Thu, 13 Feb 2025 14:49:26 -0600

Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> The changes look good. For the future now I know that to put in the
> message message, e.g. not including the news file change in the commit
> message.

It is fine to put etc/NEWS in the ChangeLog, it's just not required.

(I usually just leave it as is when installing other people's patches,
but this time I seem to have deleted the line for some reason.)

> The change "Make sieve-manage prompt also for port number" is missing
> the file name. That's ok?

No, that was an oversight on my part.  I thought I fixed it, but
apparently not.

I recommend 'C-x 4 a' or 'M-x magit-generate-changelog' to generate
pChangeLog entries in the future, that way you always get them right.
(I assume that you didn't because there was a typo in a file name in one
of the patches, which usually suggests that it was typed out manually.)

Previous Next

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