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

From: Björn Bidar <[email-masked]>
Date: Tue, 11 Feb 2025 17:44:24 +0200

Stefan Kangas <[email-masked]> writes:

> Björn Bidar via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <[email-masked]> writes:
>
>> 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.
>
> 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?= <[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.
>
> 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.