bug#43129: 25.2; Typo in lisp/gnus/nnimap.el

[Eric Abrahamsen]

Sean Connor <sconnor@allyinics.org> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Thanks for this report! Can you tell us which IMAP servers you've
>> tested this on? I just tried Dovecot, and the "(cadr result)" fix works
>> properly there. Unless we know there are some servers where "(caddr
>> result)" is appropriate (I wonder what server Nikolaus was using), I'm
>> inclined to put the simpler fix in.
>
> I was a bit mistaken.  My initial tests were on an old server that
> didn't support MOVE, so I overlooked something important, the reason for
> the change: the COPYUID is given as an "untagged response" for MOVE but
> a "tagged response" for COPY [0].  IOW, caddr is what to use for getting
> the COPYUID from a response to a MOVE command.
>
> The COPYUID response is given by both the COPY and MOVE commands.  I'd
> only been testing the COPY command, oops.

And my dovecot test was done in a clean environment where, by default,
Gnus doesn't let dovecot use MOVE, so in effect I was only testing COPY
as well. Looks like cautious is the way to go!

> The cautious patch seems to handle both cases, according to this test
> code:
>
> (mapcar (lambda (imap-response)
>     (with-temp-buffer
>       (insert imap-response)
>       (or
>        (nnimap-find-uid-response
>       "COPYUID" (cadr
>                  ;; simulate a call to nnimap-command.
>                  (cons t (nnimap-parse-response))))
>        (nnimap-find-uid-response
>       "COPYUID" (caddr (cons t (nnimap-parse-response)))))))
>  '(
>    ;; MOVE result
>    "* OK [COPYUID 1598849953 2 3] Moved UIDs.\r
> * 1 EXPUNGE\r
> 1 OK Move completed (0.015 + 0.000 + 0.014 secs).\r"
>    ;; COPY result
>    "6 OK [COPYUID 1395967160 10 3] Copy completed."))

Thanks, this is helpful. I feel like the "outer edges" of execution is
the wrong place to be checking this stuff -- if the response parsing
process handled the differences, this would never have been an issue in
the first place. And ugh, this can-move thing is all over the place, and
begging to be refactored...

But! We will not be drawn down that rabbit hole. It seems to me that
changing the code to read:

(cons internal-move-group
      (or (nnimap-find-uid-response "COPYUID"
                                    (if can-move
                                        (caddr result)
                                      (cadr result)))
          (nnimap-find-article-by-message-id
           internal-move-group server message-id
           nnimap-request-articles-find-limit)))

Should do the trick here. What do you think?

> Sorry for the confusion. This problem is only going to affect those
> whose IMAP servers don't support the MOVE extension, which is probably
> why it was overlooked.
>
> I tested with courier, dovecot, RFC sample sessions and gmail IMAP
> transcripts, FWIW.

That's good to know! That gives us some confidence.