Re: Duplicate In-Reply-To entries in reply buffer

[Kazuhiro Ito <kzhr@d1.dion.ne.jp>]

> > I think we can't simply use elmo-msgdb-get-message-id-from-buffer
> > instead of (std11-field-body "message-id").  Because,
> 
> > 1. elmo-msgdb-get-message-id-from-buffer uses elmo-unfold-field-body
> > (or elmo-unfold-field-body, previously), which assumes buffer is
> > narrrowed to the message header.
> 
> Technically this is correct and so does `elmo-field-body' in current
> `elmo-msgdb-get-message-id-from-buffer'. But when msgdb-get-message-id
> is called in `elmo-folder-append-buffer' the buffer is not narrowed to
> the headers neither.

Confirmed.  I think it is a bug introduced in 

https://github.com/wanderlust/wanderlust/commit/be58a96ce5e195f23a25a8991e7c3bc846622eec

It seems that elmo-msgdb-get-message-id-from-buffer is used instead of
(std11-field-body "message-id").  std11-field-body narrows by itself,
but elmo-msgdb-get-message-id-from-buffer does not.  I think it would
be better that we prepare wrapper function such as below;

(defmacro elmo-with-narrowed-to-header (&rest body)
  "Narrow region to a message header, and evaluate BODY there like `progn'."
  `(save-excursion
     (save-restriction
       (std11-narrow-to-header)
       (progn ,@body))))

Or, we can make elmo-msgdb-get-message-id-from-buffer to assume
narrowed buffer.  I will post another message to ML about performance
issue.

> A part I quite don't understand is the defalias in elmo-util.el:
> 
> ,----
> | (eval-and-compile
> |   (if (fboundp 'std11-fetch-field)
> |       (defalias 'elmo-field-body 'std11-fetch-field) ;;no narrow-to-region
> |     (defalias 'elmo-field-body 'std11-field-body)))
> `----
> 
> This code was already present with the first commit to WL repository
> back in 2000 and the only reason I could think of why one would use
> this defalias is to avoid (unnecessary) calls to narrow-to-region.

I don't know the true reason, but I think so too.


> If I see this right we are talking about two scenarios:
> 
>  (1) Obtain the message-id in order to store it in the message database
> 
>  (2) Obtain the message-id in order to create the
>      in-reply-to/references header fields
> 
> Obtaining a message-id is common to both scenarios, the difference is
> in the handling of a missing or invalid message-id. I refactored
> accordingly (see attached path or [1]) still using a very sloppy
> regexp to detect malformed message-id header.

1. At first, we should decide whether
elmo-msgdb-get-message-id-from-buffer assumes buffer is narrowed.

2. I think we can't use elmo-unfold-field-body from
elmo-get-message-id-from-buffer.  Because that function assumes to be
called from unnarrowed buffer.

If elmo-msgdb-get-message-id-from-buffer keeps assuming buffer is
narrowed, how about below code? (sorry, not tested)

(defun elmo-fetch-message-id-from-buffer ()
  (let ((msgid (elmo-unfold-field-body "message-id")))
    (when (and msgid (string-match "<.+>" msgid))
      (match-string 0 msgid))))

(defun elmo-msgdb-create-message-id ()
...)

(defun elmo-get-message-id-from-buffer ()
  (save-excursion
    (save-restriction
      (std11-narrow-to-header)
      (elmo-fetch-message-id-from-buffer))))

(defun elmo-msgdb-get-message-id-from-buffer ()
  (or (elmo-fetch-message-id-from-buffer)
      (elmo-msgdb-create-message-id)))


> > I think it would be better not using elmo-unfold-field-body for the
> > sake of speed.
> 
> If using `elmo-unfold-field-body' has a noticable and measurable
> impact on the overall performance then we need to think about a better
> way to unfold and maybe use an algorithm that is `good enough'.

I mean calling elmo-unfold-field-body is redundant in below code
irrespective of speed.  As I wrote previously, we can remove '\n' in
the next step by modifying regexp.

(defun elmo-msgdb-get-message-id-from-buffer ()
  (let ((msgid (elmo-unfold-field-body "message-id")))
    (if msgid
	(if (string-match "^[ \\t]*\\(<[^>]+>\\)[ \\t]*$" msgid)
	    (match-string 1 msgid)
...

-- 
Kazuhiro Ito