bug#54154: Emacs commit ae963e80a79f5a9184daabfc8197f211a39b136d (sieve-manage)

[Eli Zaretskii]

> From: Kai Tetzlaff <emacs+bug@tetzco.de>
> Cc: herbert@gojira.at,  larsi@gnus.org,  54154@debbugs.gnu.org
> Date: Thu, 19 Jan 2023 16:59:36 +0100
> 
> >> Yes, true. But since `sieve-manage--set-internal-buffer-properties' is
> >> used in two different places, the more elegant solution you suggested
> >> above would require duplicating the body of the function in those
> >> places. I just didn't see a better way.
> >
> > I'm not sure why you need to force the encoding of the process buffer,
> > when you already set the coding-system to be used for decoding stuff
> > from the process.  Is that really needed?
> 
> Not sure if it is really needed. But I wanted to make sure that both,
> the process buffer and the log buffer use identical settings. Otherwise,
> the content of the log buffer might be misleading.

I don't think it could mislead, but OK.

> > But if you really need this, then just make the insertion of the text
> > into the buffer you create optional: then for the process-buffer pass
> > nil as the text to insert, and you can do the with-current-buffer
> > dance only inside that function.
> 
> Sorry, you lost me there. I don't understand what you want to tell me.
> Which (optional) text in which buffer?

I meant this:

  (defun sieve-manage--set-buffer-and-append-text (buffer-name &rest args)
    (let ((existing-buffer (get-buffer buffer-name))
          new-buffer)
      (if existing-buffer
          (setq new-buffer existing-buffer)
        (setq new-buffer (get-buffer-create buffer-name)))
      (with-current-buffer new-buffer
        (when (not existing-buffer)
          (set-buffer-file-coding-system sieve-manage--coding-system)
          (setq-local after-change-functions nil)
          (buffer-disable-undo)
          ; What happened to set-buffer-multibyte?
          )
        (goto-char (point-max))
        (apply #'insert args))))

Then you can call it from sieve-manage-make-process-buffer like this:

    (sieve-manage--set-buffer-and-append-text
      (format " *sieve %s:%s*" sieve-manage-server sieve-manage-port)
      "")

i.e. with an empty string, so nothing gets inserted into the process
buffer.  Or you could instead change the signature to accept a single
&optional argument that is a list, and then you could make the last
two lines in the function above conditional on that argument being
non-nil.

> > Since you seem to be encoding and decoding to/from UTF-8 by hand in
> > sieve-manage-encode/decode, you should use 'binary' as the
> > process-codings-system for the network connection to the server, and
> > that's it.
> 
> That works. Done.
> 
> > What you should do is call sieve-manage-encode inside
> > sieve-manage-send, and count the bytes there after encoding the
> > payload.
> 
> Unfortunately, that is too late since the sent data - in case that the
> sent text may contain CRLF sequences - contains its own length. So in
> order to insert the correct length, I need to encode before sending.
> See:
> 
>     (defun sieve-manage-putscript (name content &optional buffer)
>       (with-current-buffer (or buffer (current-buffer))
>         (sieve-manage-send (format "PUTSCRIPT \"%s\" {%d+}%s%s" name
>                                    (length (sieve-manage-encode content))
>                                    sieve-manage-client-eol content))
>         (sieve-manage-parse-oknobye)))

This is because you pass both the text and the number to 'format'.
But that is not carved in stone: the "%d" part can never produce any
non-ASCII characters, so there's no need to encode it together with
CONTENT.  You could do this instead:

  (defun sieve-manage-send (command &optional payload)
    (let ((encoded (if payload (encode-coding-string payload 'utf-8-unix)))
          size cmdstr)
      (if encoded
          (setq size (format " {%d+}%s"
                             (length encoded) sieve-manage-client-eol)))
      (setq cmdstr (concat command size encoded))
      (sieve-manage--append-to-log cmdstr)
      (process-send-string sieve-manage-process cmdstr)))

And then you call this like below:

  (sieve-manage-send (format "PUTSCRIPT \"%s\"" name) content)
  (sieve-manage-send (format "HAVESPACE \"%s\" %s" name size))

I hope this clarifies my proposal.