Re: imap.el workaround for Exchange 2007

Re: imap.el workaround for Exchange 2007

[Reiner Steib]

On Thu, Dec 18 2008, Dave Love wrote:

> Although this Microsoft product is apparently at least a year past its
> sell-by date, we just got it, and it broke my mail reading (which was OK
> with the one with a sell-by date of 2003).
>
> The first problem was fixed by `imap-enable-exchange-bug-workaround'.
> However, you're only likely to find that variable when debugging the
> breakage.  Why doesn't it use `*' generally -- as that seems to be
> equivalent -- and remove the need for users to find that variable?

You meant "*:*" (i.e. the variant used if
`imap-enable-exchange-bug-workaround' is t)?  If I recall the
discussion correctly, this is much slower.
Cf. <http://thread.gmane.org/gmane.emacs.gnus.general/66635>.

> Alternatively, perhaps it should check for Exchange and set it
> (per-server?) automatically, 

This has been discussed, but nobody implemented it yet.  I also think
this would be the right thing.

> or otherwise at least provide a user variable to customize.

Making `imap-enable-exchange-bug-workaround' customizable?

> The next problem (of many, doubtless...) is an assertion failure "In
> imap-parse-body".  I get a response like this for Exchange's version of
> article
> <URL:nntp://news.gmane.org/gmane.comp.clustering.gridengine.users/13804>,
> which I get direct from the gridengine list:
>
>
>   * 161 FETCH (UID 4779 RFC822.SIZE 35507 BODY ("multipart" "signed"
>   ("boundary" "----=_Part_2607_14582877.1229556098849") NIL NIL NIL
>   -1) BODY[HEADER.FIELDS (Subject From Date Message-Id References
>   In-Reply-To Xref X-Spam-Status X-Bogosity X-CRM114-Status To
>   Newsgroups)] {725}
[...]
>   )230 OK FETCH completed.
>
> Note the `-1' where there should be an unsigned integer, says rfc 3501:
>
>   body-fields     = body-fld-param SP body-fld-id SP body-fld-desc SP
>                     body-fld-enc SP body-fld-octets
>   body-fld-octets = number
>   number          = 1*DIGIT
>                       ; Unsigned 32-bit integer
>
> The following patch against current CVS fixes it for me (see the last
> hunk).  I also fixed some doc/commentary, and I assume the KTH mail
> address is obsolete.
>
> 2008-12-18  Dave Love  <fx@gnu.org>
>
> 	* imap.el: Fix author email.  Doc fixes.
> 	(imap-parse-body): Work around assertion failure in bogus Exchange 2007
> 	reply.
>
> --- imap.el	22 Jun 2008 13:24:10 +0100	7.47
> +++ imap.el	18 Dec 2008 14:10:53 +0000	
[... fixes for docs and comments stripped ]
> @@ -2862,7 +2871,12 @@
>  	;; as the standard says.
>  	(push (or (imap-parse-nstring) "7BIT") body) ;; body-fld-enc
>  	(imap-forward)
> -	(push (imap-parse-number) body)	;; body-fld-octets
> +	;; Exchange 2007 can return -1, contrary to the spec...
> +	(if (eq (char-after) ?-)
> +	    (progn
> +	      (skip-chars-forward "-0-9")
> +	      (push nil body))
> +	  (push (imap-parse-number) body)) ;; body-fld-octets

Installed.  (IIRC you still have write access to the repository, don't
you?)

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: imap.el workaround for Exchange 2007

[Dave Love]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> You meant "*:*" (i.e. the variant used if
> `imap-enable-exchange-bug-workaround' is t)?

Oops, yes.

> If I recall the
> discussion correctly, this is much slower.
> Cf. <http://thread.gmane.org/gmane.emacs.gnus.general/66635>.

That refers to a different sequence set but anyhow see below.  I haven't
checked the traffic with anything except exchange.

>> Alternatively, perhaps it should check for Exchange and set it
>> (per-server?) automatically, 
>
> This has been discussed, but nobody implemented it yet.  I also think
> this would be the right thing.

I think the change below DTRT, as it checks for the exact problem.  I've
tested it with our exchange server.  Sorry it's conflated with some
random doc/commentary changes -- I should use darcs in parallel...  You
might want to note or zap the `fixme' comments in particular.

>> or otherwise at least provide a user variable to customize.
>
> Making `imap-enable-exchange-bug-workaround' customizable?

Yes, but that's moot if this change is OK.

> Installed.  (IIRC you still have write access to the repository, don't
> you?)

I don't know, but as my access to the Emacs repository was chopped, I
don't suppose I should be using it anyway.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 11422 bytes --]

2008-12-23  Dave Love  <fx@gnu.org>

	* imap.el: Doc fixes.
	(imap-fetch-safe): New.
	(imap-message-copyuid-1, imap-message-appenduid-1): Use it.
	(imap-parse-flag-list): Disambiguate assertion.

	* nnimap.el: Fix author address.
	(nnimap-debug): Doc fix.
	(nnimap-find-minmax-uid): Use imap-fetch-safe.

Index: nnimap.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/nnimap.el,v
retrieving revision 7.55
diff -u -F^( -r7.55 nnimap.el
--- nnimap.el	3 Dec 2008 20:33:34 -0000	7.55
+++ nnimap.el	23 Dec 2008 11:41:37 -0000
@@ -3,7 +3,7 @@
 ;; Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
 ;;   2007, 2008  Free Software Foundation, Inc.
 
-;; Author: Simon Josefsson <jas@pdc.kth.se>
+;; Author: Simon Josefsson <simon@josefsson.org>
 ;;         Jim Radford <radford@robby.caltech.edu>
 ;; Keywords: mail
 
@@ -163,6 +163,8 @@ (defcustom nnimap-split-rule nil
 before, either a function, or a list with group/regexp or
 group/function elements."
   :group 'nnimap
+  ;; fixme: doesn't allow `("my2server" ("INBOX" nnimap-split-fancy))'
+  ;; per example above.
   :type '(choice :tag "Rule type"
 		 (repeat :menu-tag "Single-server"
 			 :tag "Single-server list"
@@ -460,11 +462,17 @@ (defcustom nnimap-id nil
 		 (plist :key-type string :value-type string)))
 
 (defcustom nnimap-debug nil
-  "If non-nil, random debug spews are placed in *nnimap-debug* buffer.
+  "If non-nil, trace nnimap- functions into `nnimap-debug-buffer'.
+Uses `trace-function-background', so you can turn it off with,
+say, `untrace-all'.
+
 Note that username, passwords and other privacy sensitive
-information (such as e-mail) may be stored in the *nnimap-debug*
-buffer.  It is not written to disk, however.  Do not enable this
-variable unless you are comfortable with that."
+information (such as e-mail) may be stored in the buffer.
+It is not written to disk, however.  Do not enable this
+variable unless you are comfortable with that.
+
+This variable only takes effect when loading the `nnimap' library.
+See also `nnimap-log'."
   :group 'nnimap
   :type 'boolean)
 
@@ -555,8 +562,7 @@ (defun nnimap-find-minmax-uid (group &op
 	      (imap-mailbox-select group examine))
       (let (minuid maxuid)
 	(when (> (imap-mailbox-get 'exists) 0)
-	  (imap-fetch (if imap-enable-exchange-bug-workaround "1,*:*" "1,*")
-		      "UID" nil 'nouidfetch)
+	  (imap-fetch-safe '("1,*" . "1,*:*") "UID" nil 'nouidfetch)
 	  (imap-message-map (lambda (uid Uid)
 			      (setq minuid (if minuid (min minuid uid) uid)
 				    maxuid (if maxuid (max maxuid uid) uid)))
Index: imap.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/imap.el,v
retrieving revision 7.48
diff -u -F^( -r7.48 imap.el
--- imap.el	22 Dec 2008 22:38:50 -0000	7.48
+++ imap.el	23 Dec 2008 15:36:08 -0000
@@ -212,12 +212,12 @@ (defcustom imap-shell-program '("ssh %s 
 
 (defcustom imap-process-connection-type nil
   "*Value for `process-connection-type' to use for Kerberos4, GSSAPI and SSL.
-The `process-connection-type' variable control type of device
+The `process-connection-type' variable controls the type of device
 used to communicate with subprocesses.  Values are nil to use a
 pipe, or t or `pty' to use a pty.  The value has no effect if the
 system has no ptys or if all ptys are busy: then a pipe is used
-in any case.  The value takes effect when a IMAP server is
-opened, changing it after that has no effect."
+in any case.  The value takes effect when an IMAP server is
+opened; changing it after that has no effect."
   :version "22.1"
   :group 'imap
   :type 'boolean)
@@ -242,10 +242,13 @@ (defcustom imap-log nil
   :type 'boolean)
 
 (defcustom imap-debug nil
-  "If non-nil, random debug spews are placed in *imap-debug* buffer.
+  "If non-nil, trace imap- functions into `imap-debug-buffer'.
+Uses `trace-function-background', so you can turn it off with,
+say, `untrace-all'.
+
 Note that username, passwords and other privacy sensitive
-information (such as e-mail) may be stored in the *imap-debug*
-buffer.  It is not written to disk, however.  Do not enable this
+information (such as e-mail) may be stored in the buffer.
+It is not written to disk, however.  Do not enable this
 variable unless you are comfortable with that.
 
 This variable only takes effect when loading the `imap' library.
@@ -717,6 +720,15 @@ (defun imap-tls-open (name buffer server
 	 (process (open-tls-stream name buffer server port)))
     (when process
       (while (and (memq (process-status process) '(open run))
+		  ;; Fixme:  Per the "blue moon" comment, the
+		  ;; process/buffer handling here, and elsewhere in
+		  ;; functions which open streams, looks confused.
+		  ;; Obviously we can change buffers if a different
+		  ;; process handler kicks in from
+		  ;; `accept-process-output' or `sit-for' below, and
+		  ;; TRT seems to be to `save-buffer' around those
+		  ;; calls.  (I wonder why `sit-for' is used with a
+		  ;; non-zero wait.)  -- fx
 		  (set-buffer buffer) ;; XXX "blue moon" nntp.el bug
 		  (goto-char (point-max))
 		  (forward-line -1)
@@ -1087,7 +1099,7 @@ (defun imap-open-1 (buffer)
 	   imap-process))))
 
 (defun imap-open (server &optional port stream auth buffer)
-  "Open a IMAP connection to host SERVER at PORT returning a buffer.
+  "Open an IMAP connection to host SERVER at PORT returning a buffer.
 If PORT is unspecified, a default value is used (143 except
 for SSL which use 993).
 STREAM indicates the stream to use, see `imap-streams' for available
@@ -1726,6 +1738,7 @@ (defmacro imap-message-body (uid &option
   `(with-current-buffer (or ,buffer (current-buffer))
      (imap-message-get ,uid 'BODY)))
 
+;; Fixme:  Should this try to use CHARSET?
 (defun imap-search (predicate &optional buffer)
   (with-current-buffer (or buffer (current-buffer))
     (imap-mailbox-put 'search 'dummy)
@@ -1772,9 +1785,38 @@ (defun imap-string-to-integer (string &o
   (let ((number (string-to-number string base)))
     (if (> number most-positive-fixnum)
 	(error
-	 (format "String %s cannot be converted to a lisp integer" number))
+	 (format "String %s cannot be converted to a Lisp integer" number))
       number)))
 
+(defun imap-fetch-safe (uids props &optional receive nouidfetch buffer)
+  "Like `imap-fetch', but DTRT with Exchange 2007 bug.
+However, UIDS here is a cons, where the car is the canonical form
+of the UIDS specification, and the cdr is the one which works with
+Exchange 2007 or, potentially, other buggy servers.
+See `imap-enable-exchange-bug-workaround'."
+  ;; We don't unconditionally use the alternative (valid) form, since
+  ;; this is said to be significantly inefficient.  The first time we
+  ;; get here for a given, we'll try the canonical form.  If we get
+  ;; the known error from the buggy server, set the flag
+  ;; buffer-locally (to account for connexions to multiple servers),
+  ;; then re-try with the alternative UIDS spec.
+  (condition-case data
+      (imap-fetch (if imap-enable-exchange-bug-workaround
+		      (cdr uids)
+		    (car uids))
+		  props receive nouidfetch buffer)
+    (error
+     (if (and (not imap-enable-exchange-bug-workaround)
+	      (string-match
+	       "The specified message set is invalid"
+	       (cadr data)))
+	 (with-current-buffer (or buffer (current-buffer))
+	   (set (make-local-variable
+		 'imap-enable-exchange-bug-workaround)
+		t)
+	   (imap-fetch (cdr uids) props receive nouidfetch))
+       (signal (car data) (cdr data))))))
+
 (defun imap-message-copyuid-1 (mailbox)
   (if (imap-capability 'UIDPLUS)
       (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
@@ -1784,11 +1826,7 @@ (defun imap-message-copyuid-1 (mailbox)
 	  (imap-message-data (make-vector 2 0)))
       (when (imap-mailbox-examine-1 mailbox)
 	(prog1
-	    (and (imap-fetch
-		  ;; why the switch here, since they seem to be
-		  ;; equivalent, and ~ no-one is going to find this
-		  ;; switch?  -- fx
-		  (if imap-enable-exchange-bug-workaround "*:*" "*") "UID")
+	    (and (imap-fetch-safe '("*" . "*:*") "UID")
 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
 		       (apply 'max (imap-message-map
 				    (lambda (uid prop) uid) 'UID))))
@@ -1824,6 +1862,8 @@ (defun imap-message-copy (articles mailb
 	    (or no-copyuid
 		(imap-message-copyuid-1 mailbox)))))))
 
+;; Fixme: Amalgamate with imap-message-copyuid-1, using an extra arg,
+;; since it shares most of the code?
 (defun imap-message-appenduid-1 (mailbox)
   (if (imap-capability 'UIDPLUS)
       (imap-mailbox-get-1 'appenduid mailbox)
@@ -1832,8 +1872,7 @@ (defun imap-message-appenduid-1 (mailbox
 	  (imap-message-data (make-vector 2 0)))
       (when (imap-mailbox-examine-1 mailbox)
 	(prog1
-	    (and (imap-fetch
-		  (if imap-enable-exchange-bug-workaround "*:*" "*") "UID")
+	    (and (imap-fetch-safe '("*" "*:*") "UID")
 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
 		       (apply 'max (imap-message-map
 				    (lambda (uid prop) uid) 'UID))))
@@ -2210,7 +2249,7 @@ (defsubst imap-parse-mailbox ()
 ;;   resp-cond-bye   = "BYE" SP resp-text
 
 (defun imap-parse-greeting ()
-  "Parse a IMAP greeting."
+  "Parse an IMAP greeting."
   (cond ((looking-at "\\* OK ")
 	 (setq imap-state 'nonauth))
 	((looking-at "\\* PREAUTH ")
@@ -2255,7 +2294,7 @@ (defun imap-parse-greeting ()
 ;;                       ; capability.
 
 (defun imap-parse-response ()
-  "Parse a IMAP command response."
+  "Parse an IMAP command response."
   (let (token)
     (case (setq token (read (current-buffer)))
       (+ (setq imap-continuation
@@ -2637,7 +2676,7 @@ (defun imap-parse-flag-list ()
 			      (point)))
 		(> (skip-chars-forward "^ )" (point-at-eol)) 0))
       (push (buffer-substring start (point)) flag-list))
-    (assert (eq (char-after) ?\)) nil "In imap-parse-flag-list")
+    (assert (eq (char-after) ?\)) nil "In imap-parse-flag-list 2")
     (imap-forward)
     (nreverse flag-list)))
 
@@ -2866,9 +2905,10 @@ (defun imap-parse-body ()
 	(imap-forward)
 	(push (imap-parse-nstring) body) ;; body-fld-desc
 	(imap-forward)
-	;; next `or' for Sun SIMS bug, it regard body-fld-enc as a
-	;; nstring and return nil instead of defaulting back to 7BIT
+	;; Next `or' for Sun SIMS bug.  It regards body-fld-enc as a
+	;; nstring and returns nil instead of defaulting back to 7BIT
 	;; as the standard says.
+	;; Exchange (2007, at least) does this as well.
 	(push (or (imap-parse-nstring) "7BIT") body) ;; body-fld-enc
 	(imap-forward)
 	;; Exchange 2007 can return -1, contrary to the spec...
@@ -2878,15 +2918,15 @@ (defun imap-parse-body ()
 	      (push nil body))
 	  (push (imap-parse-number) body)) ;; body-fld-octets
 
-   ;; ok, we're done parsing the required parts, what comes now is one
+	;; ok, we're done parsing the required parts, what comes now is one
 	;; of three things:
 	;;
 	;; envelope       (then we're parsing body-type-msg)
 	;; body-fld-lines (then we're parsing body-type-text)
 	;; body-ext-1part (then we're parsing body-type-basic)
 	;;
-  ;; the problem is that the two first are in turn optionally followed
-;; by the third.  So we parse the first two here (if there are any)...
+	;; the problem is that the two first are in turn optionally followed
+	;; by the third.  So we parse the first two here (if there are any)...
 
 	(when (eq (char-after) ?\ )
 	  (imap-forward)

Re: imap.el workaround for Exchange 2007

[Reiner Steib]

On Tue, Dec 23 2008, Dave Love wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> writes:
[...]
>>> Alternatively, perhaps it should check for Exchange and set it
>>> (per-server?) automatically, 
>>
>> This has been discussed, but nobody implemented it yet.  I also think
>> this would be the right thing.
>
> I think the change below DTRT, as it checks for the exact problem.  I've
> tested it with our exchange server.  

I've committed the `imap-fetch-safe' hunks:
<http://article.gmane.org/gmane.emacs.gnus.commits/6100>

> Sorry it's conflated with some random doc/commentary changes -- I
> should use darcs in parallel...  You might want to note or zap the
> `fixme' comments in particular.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: imap.el workaround for Exchange 2007

[Ted Zlatanov]

On Thu, 01 Jan 2009 19:34:48 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 

RS> On Tue, Dec 23 2008, Dave Love wrote:
>> Reiner Steib <reinersteib+gmane@imap.cc> writes:
RS> [...]
>>>> Alternatively, perhaps it should check for Exchange and set it
>>>> (per-server?) automatically, 
>>> 
>>> This has been discussed, but nobody implemented it yet.  I also think
>>> this would be the right thing.
>> 
>> I think the change below DTRT, as it checks for the exact problem.  I've
>> tested it with our exchange server.  

RS> I've committed the `imap-fetch-safe' hunks:
RS> <http://article.gmane.org/gmane.emacs.gnus.commits/6100>

Should we deprecate the imap exchange bug workaround variable now that
this is automatically handled?

Ted

Re: imap.el workaround for Exchange 2007

[Reiner Steib]

On Fri, Jan 02 2009, Ted Zlatanov wrote:

> On Thu, 01 Jan 2009 19:34:48 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 
>
> RS> On Tue, Dec 23 2008, Dave Love wrote:
>>> I think the change below DTRT, as it checks for the exact problem.  I've
>>> tested it with our exchange server.  
>
> RS> I've committed the `imap-fetch-safe' hunks:
> RS> <http://article.gmane.org/gmane.emacs.gnus.commits/6100>
>
> Should we deprecate the imap exchange bug workaround variable now that
> this is automatically handled?

The variable is still used.  But maybe its doc string should be
updated.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: imap.el workaround for Exchange 2007

[Simon Josefsson]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> On Tue, Dec 23 2008, Dave Love wrote:
>
>> Reiner Steib <reinersteib+gmane@imap.cc> writes:
> [...]
>>>> Alternatively, perhaps it should check for Exchange and set it
>>>> (per-server?) automatically, 
>>>
>>> This has been discussed, but nobody implemented it yet.  I also think
>>> this would be the right thing.
>>
>> I think the change below DTRT, as it checks for the exact problem.  I've
>> tested it with our exchange server.  
>
> I've committed the `imap-fetch-safe' hunks:
> <http://article.gmane.org/gmane.emacs.gnus.commits/6100>

I like the approach, thanks.

/Simon

FIXMEs in imap.el and nnimap.el (was: imap.el workaround for Exchange 2007)

[Reiner Steib]

On Tue, Dec 23 2008, Dave Love wrote:

> Sorry it's conflated with some random doc/commentary changes -- I
> should use darcs in parallel...  You might want to note or zap the
> `fixme' comments in particular.

Thanks, I've committed them with minor editing (sorry that this will
cause merge conflicts for you):
<http://article.gmane.org/gmane.emacs.gnus.commits/6101>

Would someone take a look at the FIXMEs, please?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: FIXMEs in imap.el and nnimap.el

[Ted Zlatanov]

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

On Thu, 01 Jan 2009 19:55:36 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 

RS> On Tue, Dec 23 2008, Dave Love wrote:
>> Sorry it's conflated with some random doc/commentary changes -- I
>> should use darcs in parallel...  You might want to note or zap the
>> `fixme' comments in particular.

RS> Thanks, I've committed them with minor editing (sorry that this will
RS> cause merge conflicts for you):
RS> <http://article.gmane.org/gmane.emacs.gnus.commits/6101>

RS> Would someone take a look at the FIXMEs, please?

I don't know about the rest of them, but the easy patch, which merges
copyuid and appenduid handling, is appended below.  It's really trivial
and the code is almost exactly the same so if someone wants to try it to
verify it works, I'll commit it.

Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: copyuid+appenduid.merge.patch --]
[-- Type: text/x-diff, Size: 2844 bytes --]

Index: imap.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/imap.el,v
retrieving revision 7.50
diff -c -r7.50 imap.el
*** imap.el	1 Jan 2009 18:50:34 -0000	7.50
--- imap.el	2 Jan 2009 22:14:42 -0000
***************
*** 1816,1830 ****
         (signal (car data) (cdr data))))))
  
  (defun imap-message-copyuid-1 (mailbox)
    (if (imap-capability 'UIDPLUS)
!       (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
! 	    (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
      (let ((old-mailbox imap-current-mailbox)
  	  (state imap-state)
  	  (imap-message-data (make-vector 2 0)))
        (when (imap-mailbox-examine-1 mailbox)
  	(prog1
! 	    (and (imap-fetch-safe '("*" . "*:*") "UID")
  		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
  		       (apply 'max (imap-message-map
  				    (lambda (uid prop) uid) 'UID))))
--- 1816,1838 ----
         (signal (car data) (cdr data))))))
  
  (defun imap-message-copyuid-1 (mailbox)
+   (imap-message-copyuid-or-appenduid-1 mailbox t))
+ 
+ (defun imap-message-appenduid-1 (mailbox)
+   (imap-message-copyuid-or-appenduid-1 mailbox nil))
+ 
+ (defun imap-message-copyuid-or-appenduid-1 (mailbox copy)
    (if (imap-capability 'UIDPLUS)
!       (if copy
! 	  (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
! 		(string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
! 	(imap-mailbox-get-1 'appenduid mailbox))
      (let ((old-mailbox imap-current-mailbox)
  	  (state imap-state)
  	  (imap-message-data (make-vector 2 0)))
        (when (imap-mailbox-examine-1 mailbox)
  	(prog1
! 	    (and (imap-fetch-safe '("*" "*:*") "UID")
  		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
  		       (apply 'max (imap-message-map
  				    (lambda (uid prop) uid) 'UID))))
***************
*** 1860,1883 ****
  	    (or no-copyuid
  		(imap-message-copyuid-1 mailbox)))))))
  
- ;; FIXME: Amalgamate with imap-message-copyuid-1, using an extra arg, since it
- ;; shares most of the code?  -- fx
- (defun imap-message-appenduid-1 (mailbox)
-   (if (imap-capability 'UIDPLUS)
-       (imap-mailbox-get-1 'appenduid mailbox)
-     (let ((old-mailbox imap-current-mailbox)
- 	  (state imap-state)
- 	  (imap-message-data (make-vector 2 0)))
-       (when (imap-mailbox-examine-1 mailbox)
- 	(prog1
- 	    (and (imap-fetch-safe '("*" "*:*") "UID")
- 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
- 		       (apply 'max (imap-message-map
- 				    (lambda (uid prop) uid) 'UID))))
- 	  (if old-mailbox
- 	      (imap-mailbox-select old-mailbox (eq state 'examine))
- 	    (imap-mailbox-unselect)))))))
- 
  (defun imap-message-appenduid (mailbox &optional buffer)
    (with-current-buffer (or buffer (current-buffer))
      (imap-message-appenduid-1 (imap-utf7-encode mailbox))))
--- 1868,1873 ----

Re: FIXMEs in imap.el and nnimap.el

[Reiner Steib]

On Fri, Jan 02 2009, Ted Zlatanov wrote:

> On Thu, 01 Jan 2009 19:55:36 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 
> RS> Thanks, I've committed them with minor editing (sorry that this will
> RS> cause merge conflicts for you):
> RS> <http://article.gmane.org/gmane.emacs.gnus.commits/6101>
>
> RS> Would someone take a look at the FIXMEs, please?
>
> I don't know about the rest of them, but the easy patch, which merges
> copyuid and appenduid handling, is appended below.  It's really trivial
> and the code is almost exactly the same so if someone wants to try it to
> verify it works, I'll commit it.

I'm confused about '("*" . "*:*") vs. '("*" "*:*").  Shouldn't it be
the former in both functions, Dave?

--- /tmp/ediff2426614a	2009-01-02 23:42:49.000000000 +0100
+++ /tmp/ediff24266ouU	2009-01-02 23:42:49.000000000 +0100
@@ -1,13 +1,12 @@
-(defun imap-message-copyuid-1 (mailbox)
+(defun imap-message-appenduid-1 (mailbox)
   (if (imap-capability 'UIDPLUS)
-      (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
-	    (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
+      (imap-mailbox-get-1 'appenduid mailbox)
     (let ((old-mailbox imap-current-mailbox)
 	  (state imap-state)
 	  (imap-message-data (make-vector 2 0)))
       (when (imap-mailbox-examine-1 mailbox)
 	(prog1
-	    (and (imap-fetch-safe '("*" . "*:*") "UID")
+	    (and (imap-fetch-safe '("*" "*:*") "UID")
 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
 		       (apply 'max (imap-message-map
 				    (lambda (uid prop) uid) 'UID))))

> Index: imap.el
> ===================================================================
> RCS file: /usr/local/cvsroot/gnus/lisp/imap.el,v
> retrieving revision 7.50
> diff -c -r7.50 imap.el
> *** imap.el	1 Jan 2009 18:50:34 -0000	7.50
> --- imap.el	2 Jan 2009 22:14:42 -0000
> ***************
> *** 1816,1830 ****
>          (signal (car data) (cdr data))))))
>   
>   (defun imap-message-copyuid-1 (mailbox)
>     (if (imap-capability 'UIDPLUS)
> !       (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
> ! 	    (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
>       (let ((old-mailbox imap-current-mailbox)
>   	  (state imap-state)
>   	  (imap-message-data (make-vector 2 0)))
>         (when (imap-mailbox-examine-1 mailbox)
>   	(prog1
> ! 	    (and (imap-fetch-safe '("*" . "*:*") "UID")
>   		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
>   		       (apply 'max (imap-message-map
>   				    (lambda (uid prop) uid) 'UID))))
> --- 1816,1838 ----
>          (signal (car data) (cdr data))))))
>   
>   (defun imap-message-copyuid-1 (mailbox)
> +   (imap-message-copyuid-or-appenduid-1 mailbox t))
> + 
> + (defun imap-message-appenduid-1 (mailbox)
> +   (imap-message-copyuid-or-appenduid-1 mailbox nil))
> + 
> + (defun imap-message-copyuid-or-appenduid-1 (mailbox copy)
>     (if (imap-capability 'UIDPLUS)
> !       (if copy

I'd rather use
  (defun imap-message-copyuid-1 (mailbox &optional append)
or 
  (defun imap-message-appenduid-1 (mailbox &optional copy)
and adjust the callers.

> ! 	  (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
> ! 		(string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
> ! 	(imap-mailbox-get-1 'appenduid mailbox))
>       (let ((old-mailbox imap-current-mailbox)
>   	  (state imap-state)
>   	  (imap-message-data (make-vector 2 0)))
>         (when (imap-mailbox-examine-1 mailbox)
>   	(prog1
> ! 	    (and (imap-fetch-safe '("*" "*:*") "UID")
>   		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
>   		       (apply 'max (imap-message-map
>   				    (lambda (uid prop) uid) 'UID))))
> ***************
> *** 1860,1883 ****
>   	    (or no-copyuid
>   		(imap-message-copyuid-1 mailbox)))))))
>   
> - ;; FIXME: Amalgamate with imap-message-copyuid-1, using an extra arg, since it
> - ;; shares most of the code?  -- fx
> - (defun imap-message-appenduid-1 (mailbox)
> -   (if (imap-capability 'UIDPLUS)
> -       (imap-mailbox-get-1 'appenduid mailbox)
> -     (let ((old-mailbox imap-current-mailbox)
> - 	  (state imap-state)
> - 	  (imap-message-data (make-vector 2 0)))
> -       (when (imap-mailbox-examine-1 mailbox)
> - 	(prog1
> - 	    (and (imap-fetch-safe '("*" "*:*") "UID")
> - 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
> - 		       (apply 'max (imap-message-map
> - 				    (lambda (uid prop) uid) 'UID))))
> - 	  (if old-mailbox
> - 	      (imap-mailbox-select old-mailbox (eq state 'examine))
> - 	    (imap-mailbox-unselect)))))))
> - 
>   (defun imap-message-appenduid (mailbox &optional buffer)
>     (with-current-buffer (or buffer (current-buffer))
>       (imap-message-appenduid-1 (imap-utf7-encode mailbox))))
> --- 1868,1873 ----
>

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: FIXMEs in imap.el and nnimap.el

[Dave Love]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> I'm confused about '("*" . "*:*") vs. '("*" "*:*").  Shouldn't it be
> the former in both functions, Dave?

Certainly my version of imap-fetch-safe will fail if it's not called
with a pair.

Re: FIXMEs in imap.el and nnimap.el

[Dave Love]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

I wrote:

> Certainly my version of imap-fetch-safe will fail if it's not called
> with a pair.

Sorry, I was confused by the context and just realized that the typo is
in my code originally.  I don't understand that, because (a) I thought
I'd fixed it before sending the patch, and (b) I thought I was running
with a copy up-to-date with CVS...  Anyhow, this is right:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 724 bytes --]

2009-01-07  Dave Love  <fx@gnu.org>

	* imap.el (imap-message-appenduid-1): Fix typo in imap-fetch-safe call.

Index: imap.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/imap.el,v
retrieving revision 7.50
diff -u -r7.50 imap.el
--- imap.el	1 Jan 2009 18:50:34 -0000	7.50
+++ imap.el	7 Jan 2009 20:49:30 -0000
@@ -1870,7 +1870,7 @@
 	  (imap-message-data (make-vector 2 0)))
       (when (imap-mailbox-examine-1 mailbox)
 	(prog1
-	    (and (imap-fetch-safe '("*" "*:*") "UID")
+	    (and (imap-fetch-safe '("*" . "*:*") "UID")
 		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
 		       (apply 'max (imap-message-map
 				    (lambda (uid prop) uid) 'UID))))

Re: FIXMEs in imap.el and nnimap.el

[Reiner Steib]

On Wed, Jan 07 2009, Dave Love wrote:

> 2009-01-07  Dave Love  <fx@gnu.org>
>
> 	* imap.el (imap-message-appenduid-1): Fix typo in imap-fetch-safe call.

Thanks for clarifying.  It has been fixed already after this report:
http://thread.gmane.org/gmane.emacs.gnus.general/68095/focus=68098
<news:b4meizfo5cz.fsf@jpl.org>

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: FIXMEs in imap.el and nnimap.el

[Ted Zlatanov]

On Fri, 02 Jan 2009 23:57:08 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 

>> (defun imap-message-copyuid-1 (mailbox)
>> +   (imap-message-copyuid-or-appenduid-1 mailbox t))
>> + 
>> + (defun imap-message-appenduid-1 (mailbox)
>> +   (imap-message-copyuid-or-appenduid-1 mailbox nil))
>> + 
>> + (defun imap-message-copyuid-or-appenduid-1 (mailbox copy)
>> (if (imap-capability 'UIDPLUS)
>> !       (if copy

RS> I'd rather use
RS>   (defun imap-message-copyuid-1 (mailbox &optional append)
RS> or 
RS>   (defun imap-message-appenduid-1 (mailbox &optional copy)
RS> and adjust the callers.

Why?  The two functions are clearly mapped to separate IMAP concepts, so
merging them and requiring all the callers to adjust is unnecessary
pain.

The other issue (cons cell vs. list for "*" "*:*") I'll leave to Dave.

Ted

Re: FIXMEs in imap.el and nnimap.el

[Reiner Steib]

On Wed, Jan 07 2009, Ted Zlatanov wrote:

> On Fri, 02 Jan 2009 23:57:08 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 
>
>>> (defun imap-message-copyuid-1 (mailbox)
>>> +   (imap-message-copyuid-or-appenduid-1 mailbox t))
>>> + 
>>> + (defun imap-message-appenduid-1 (mailbox)
>>> +   (imap-message-copyuid-or-appenduid-1 mailbox nil))
>>> + 
>>> + (defun imap-message-copyuid-or-appenduid-1 (mailbox copy)
>>> (if (imap-capability 'UIDPLUS)
>>> !       (if copy
>
> RS> I'd rather use
> RS>   (defun imap-message-copyuid-1 (mailbox &optional append)
> RS> or 
> RS>   (defun imap-message-appenduid-1 (mailbox &optional copy)
> RS> and adjust the callers.
>
> Why?  The two functions are clearly mapped to separate IMAP concepts, 

Dunno, I'm not familiar with those.

> so merging them and requiring all the callers to adjust is
> unnecessary pain.

The number of callers is very limited: imap-message-appenduid and
imap-message-append.

I'm not sure if the later should in fact call imap-message-appenduid
instead of imap-message-appenduid-1 (see its use of imap-utf7-encode).

OTOH, keeping imap-message-appenduid-1 as a simple wrapper won't hurt.

How about this?

--- imap.el	08 Jan 2009 21:49:23 +0100	7.52
+++ imap.el	08 Jan 2009 21:55:02 +0100	
@@ -1821,10 +1821,12 @@
 	   (imap-fetch (cdr uids) props receive nouidfetch))
        (signal (car data) (cdr data))))))
 
-(defun imap-message-copyuid-1 (mailbox)
+(defun imap-message-copyuid-1 (mailbox &optional append)
   (if (imap-capability 'UIDPLUS)
-      (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
-	    (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
+      (if append
+	  (imap-mailbox-get-1 'appenduid mailbox)
+	(list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
+	      (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox)))))
     (let ((old-mailbox imap-current-mailbox)
 	  (state imap-state)
 	  (imap-message-data (make-vector 2 0)))
@@ -1866,23 +1868,8 @@
 	    (or no-copyuid
 		(imap-message-copyuid-1 mailbox)))))))
 
-;; FIXME: Amalgamate with imap-message-copyuid-1, using an extra arg, since it
-;; shares most of the code?  -- fx
 (defun imap-message-appenduid-1 (mailbox)
-  (if (imap-capability 'UIDPLUS)
-      (imap-mailbox-get-1 'appenduid mailbox)
-    (let ((old-mailbox imap-current-mailbox)
-	  (state imap-state)
-	  (imap-message-data (make-vector 2 0)))
-      (when (imap-mailbox-examine-1 mailbox)
-	(prog1
-	    (and (imap-fetch-safe '("*" . "*:*") "UID")
-		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
-		       (apply 'max (imap-message-map
-				    (lambda (uid prop) uid) 'UID))))
-	  (if old-mailbox
-	      (imap-mailbox-select old-mailbox (eq state 'examine))
-	    (imap-mailbox-unselect)))))))
+  (imap-message-copyuid-1 mailbox t))
 
 (defun imap-message-appenduid (mailbox &optional buffer)
   (with-current-buffer (or buffer (current-buffer))


Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: FIXMEs in imap.el and nnimap.el

[Dave Love]

I'm afraid there's something wrong with my fix, but it seems to be
something like a race condition, and isn't consistently reproducible.
It also goes away when I try to debug it.  I see the old error sometimes
when starting Gnus afresh, and the trace says imap-fetch-safe is called
from nnimap-find-minmax-uid.  It appears that the condition-case in
imap-fetch-safe isn't catching the error thrown by imap-parse-message.  

Now I think about it, I'm more puzzled, and obviously don't understand
the logic.  An error in a process filter should be caught anyway, so I
wonder why the code ever works, though it does some of the time.  Any
ideas?

Setting imap-enable-exchange-bug-workaround globally works, of course,
and it's not clear to me what the problem is with always using the
workaround anyhow.  Are there servers on which it's known to fail or be
noticeably slow?

IMAP and Exchange 2007 - imap-fetch-safe (was: FIXMEs in imap.el and nnimap.el)

[Reiner Steib]

On Tue, Jan 13 2009, Dave Love wrote:

> I'm afraid there's something wrong with my fix, but it seems to be
> something like a race condition, and isn't consistently reproducible.

Bjorn Solberg reported that the fix wont work for him too:
See <news:m3bpud5mmy.fsf@famsolberg.com> on ding, or 
http://thread.gmane.org/gmane.emacs.gnus.general/68106/focus=68138
where he posted the *imap-debug*" buffer.

> It also goes away when I try to debug it.  I see the old error sometimes
> when starting Gnus afresh, and the trace says imap-fetch-safe is called
> from nnimap-find-minmax-uid.  It appears that the condition-case in
> imap-fetch-safe isn't catching the error thrown by imap-parse-message.  
>
> Now I think about it, I'm more puzzled, and obviously don't understand
> the logic.  An error in a process filter should be caught anyway, so I
> wonder why the code ever works, though it does some of the time.  Any
> ideas?

Maybe someone on emacs-devel has an idea (Cc-ed).  For emacs-devel: we
are talking about the recent addition `imap-fetch-safe' in `imap.el'
and `nnimap.el'.  AFAICS, the code in the Emacs trunk is uptodate.
Note that Dave uses Emacs 21, Bjorn uses Emacs/23.0.50 according to
his headers.

> Setting imap-enable-exchange-bug-workaround globally works, of course,

It should also be possible to set it as a server variable
(info "(gnus)Server Variables").

> and it's not clear to me what the problem is with always using the
> workaround anyhow.  Are there servers on which it's known to fail or be
> noticeably slow?

Simon?  Others?
http://thread.gmane.org/gmane.emacs.gnus.general/66635/focus=94117

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Simon Josefsson]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

>> It also goes away when I try to debug it.  I see the old error sometimes
>> when starting Gnus afresh, and the trace says imap-fetch-safe is called
>> from nnimap-find-minmax-uid.  It appears that the condition-case in
>> imap-fetch-safe isn't catching the error thrown by imap-parse-message.  

Maybe the problem is a condition-case within another condition-case
work?  I recall problems related to this when run as an async process
filter.

Maybe it is possible to re-write the approach without using
condition-case, that would likely be easier to debug anyway.

>> and it's not clear to me what the problem is with always using the
>> workaround anyhow.  Are there servers on which it's known to fail or be
>> noticeably slow?
>
> Simon?  Others?
> http://thread.gmane.org/gmane.emacs.gnus.general/66635/focus=94117

I'd prefer to avoid sending the Exchange bug-workaround approach
("1,*:*") to any server that does not need it.  I've seen servers that
(internally) open up all e-mails in the folder and searches them, but
for the 1,* approach was able to return data quickly.  This may be old
information now, but generally I don't see why imap.el should send poor
protocol output to all servers just because Exchange is broken.

/Simon

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Dave Love]

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

Simon Josefsson <simon@josefsson.org> writes:

> Maybe the problem is a condition-case within another condition-case
> work?  I recall problems related to this when run as an async process
> filter.

Yes -- signals from process filters are normally caught; I wasn't
thinking or looking closely enough at what the code does originally.  I
must have had debug-on-error set when testing, but I was sure I'd
actually tried it in fresh Emacs.

> Maybe it is possible to re-write the approach without using
> condition-case, that would likely be easier to debug anyway.

I'm not sure it would be easier, but see the comment in the patch below.
I made it before reading this, and at least the fix over my previous
code is just an extra binding.  It works for me in a fresh Emacs, and
isn't broken in Emacs 22.

> I'd prefer to avoid sending the Exchange bug-workaround approach
> ("1,*:*") to any server that does not need it.  I've seen servers that
> (internally) open up all e-mails in the folder and searches them, but
> for the 1,* approach was able to return data quickly.

Obviously that's a good reason.  Previous comments about efficiency that
I was referred to seemed to be about something different.

> This may be old
> information now, but generally I don't see why imap.el should send poor
> protocol output to all servers just because Exchange is broken.

[It does have workarounds for various other servers, not that I want to
defend Exchange in any way.  Exchange 2007 is doing horrible things like
messing with MIME parts, which I don't think the previous version did,
and I wish I could avoid it.]

Anyhow, per the comment in the patch, is there a good reason -- other
than simplicity? -- to use FETCH rather than UID like other clients?
I'm speaking mostly in ignorance of IMAP...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3149 bytes --]

2009-01-17  Dave Love  <fx@gnu.org>

	* imap.el (imap-fetch-safe): Bind debug-on-error.
	(imap-debug): Add imap-fetch-safe.

Index: imap.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/imap.el,v
retrieving revision 7.52
diff -u -r7.52 imap.el
--- imap.el	8 Jan 2009 20:51:29 -0000	7.52
+++ imap.el	17 Jan 2009 19:33:31 -0000
@@ -1798,25 +1800,38 @@
 of the UIDS specification, and the cdr is the one which works with
 Exchange 2007 or, potentially, other buggy servers.
 See `imap-enable-exchange-bug-workaround'."
-  ;; We don't unconditionally use the alternative (valid) form, since
-  ;; this is said to be significantly inefficient.  The first time we
-  ;; get here for a given, we'll try the canonical form.  If we get
-  ;; the known error from the buggy server, set the flag
-  ;; buffer-locally (to account for connections to multiple servers),
-  ;; then re-try with the alternative UIDS spec.
+  ;; The first time we get here for a given, we'll try the canonical
+  ;; form.  If we get the known error from the buggy server, set the
+  ;; flag buffer-locally (to account for connections to multiple
+  ;; servers), then re-try with the alternative UIDS spec.  We don't
+  ;; unconditionally use the alternative form, since the
+  ;; currently-used alternatives are seriously inefficient with some
+  ;; servers (although they are valid).
+  ;;
+  ;; FIXME:  Maybe it would be cleaner to have a flag to not signal
+  ;; the error (which otherwise gives a message), and test
+  ;; `imap-failed-tags'.  Also, Other IMAP clients use other forms of
+  ;; request which work with Exchange, e.g. Claws does "UID FETCH 1:*
+  ;; (UID)" rather than "FETCH UID 1,*".  Is there a good reason not
+  ;; to do the same?
   (condition-case data
-      (imap-fetch (if imap-enable-exchange-bug-workaround
-		      (cdr uids)
-		    (car uids))
-		  props receive nouidfetch buffer)
+      ;; Binding `debug-on-error' allows us to get the error from
+      ;; `imap-parse-response' -- it's normally caught by Emacs around
+      ;; execution of a process filter.
+      (let ((debug-on-error t))
+	(imap-fetch (if imap-enable-exchange-bug-workaround
+			(cdr uids)
+		      (car uids))
+		    props receive nouidfetch buffer))
     (error
      (if (and (not imap-enable-exchange-bug-workaround)
-	      (string-match
-	       "The specified message set is invalid"
-	       (cadr data)))
+	      ;; This is the Exchange 2007 response.  It may be more
+	      ;; robust just to check for a BAD response to the
+	      ;; attempted fetch.
+	      (string-match "The specified message set is invalid"
+			    (cadr data)))
 	 (with-current-buffer (or buffer (current-buffer))
-	   (set (make-local-variable
-		 'imap-enable-exchange-bug-workaround)
+	   (set (make-local-variable 'imap-enable-exchange-bug-workaround)
 		t)
 	   (imap-fetch (cdr uids) props receive nouidfetch))
        (signal (car data) (cdr data))))))
@@ -3023,6 +3038,7 @@
 	  imap-list-to-message-set
 	  imap-fetch-asynch
 	  imap-fetch
+	  imap-fetch-safe
 	  imap-message-put
 	  imap-message-get
 	  imap-message-map

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Dave Love]

Could someone please apply the patch in the previous message in this
thread, or tell me what needs fixing in it?  As far as I can tell, it's
working OK.  Ta.

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Reiner Steib]

On Sat, Jan 17 2009, Dave Love wrote:

> Simon Josefsson <simon@josefsson.org> writes:
>
>> Maybe the problem is a condition-case within another condition-case
>> work?  I recall problems related to this when run as an async process
>> filter.
>
> Yes -- signals from process filters are normally caught; I wasn't
> thinking or looking closely enough at what the code does originally.  I
> must have had debug-on-error set when testing, but I was sure I'd
> actually tried it in fresh Emacs.
>
>> Maybe it is possible to re-write the approach without using
>> condition-case, that would likely be easier to debug anyway.
>
> I'm not sure it would be easier, but see the comment in the patch below.
> I made it before reading this, and at least the fix over my previous
> code is just an extra binding.  It works for me in a fresh Emacs, and
> isn't broken in Emacs 22.
>
>> I'd prefer to avoid sending the Exchange bug-workaround approach
>> ("1,*:*") to any server that does not need it.  I've seen servers that
>> (internally) open up all e-mails in the folder and searches them, but
>> for the 1,* approach was able to return data quickly.
>
> Obviously that's a good reason.  Previous comments about efficiency that
> I was referred to seemed to be about something different.
>
>> This may be old information now, but generally I don't see why
>> imap.el should send poor protocol output to all servers just
>> because Exchange is broken.
>
> [It does have workarounds for various other servers, not that I want to
> defend Exchange in any way.  Exchange 2007 is doing horrible things like
> messing with MIME parts, which I don't think the previous version did,
> and I wish I could avoid it.]
>
> Anyhow, per the comment in the patch, 

+  ;; FIXME:  Maybe it would be cleaner to have a flag to not signal
+  ;; the error (which otherwise gives a message), and test
+  ;; `imap-failed-tags'.  Also, Other IMAP clients use other forms of
+  ;; request which work with Exchange, e.g. Claws does "UID FETCH 1:*
+  ;; (UID)" rather than "FETCH UID 1,*".  Is there a good reason not
+  ;; to do the same?

> is there a good reason -- other than simplicity? -- to use FETCH
> rather than UID like other clients?  I'm speaking mostly in
> ignorance of IMAP...

Comments?  (Simon?)

> 2009-01-17  Dave Love  <fx@gnu.org>
>
> 	* imap.el (imap-fetch-safe): Bind debug-on-error.
> 	(imap-debug): Add imap-fetch-safe.

Installed.

Bjorn (and others), does the auto-detection work for you with the
current version from CVS?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Bjorn Solberg]

Reiner Steib writes:

> On Sat, Jan 17 2009, Dave Love wrote:
>> Simon Josefsson <simon@josefsson.org> writes:
>> 
>>> Maybe the problem is a condition-case within another condition-case
>>> work?  I recall problems related to this when run as an async process
>>> filter.
>> 
>> Yes -- signals from process filters are normally caught; I wasn't
>> thinking or looking closely enough at what the code does originally.  I
>> must have had debug-on-error set when testing, but I was sure I'd
>> actually tried it in fresh Emacs.
>> 
>>> Maybe it is possible to re-write the approach without using
>>> condition-case, that would likely be easier to debug anyway.
>> 
>> I'm not sure it would be easier, but see the comment in the patch below.
>> I made it before reading this, and at least the fix over my previous
>> code is just an extra binding.  It works for me in a fresh Emacs, and
>> isn't broken in Emacs 22.
>> 
>>> I'd prefer to avoid sending the Exchange bug-workaround approach
>>> ("1,*:*") to any server that does not need it.  I've seen servers that
>>> (internally) open up all e-mails in the folder and searches them, but
>>> for the 1,* approach was able to return data quickly.
>> 
>> Obviously that's a good reason.  Previous comments about efficiency that
>> I was referred to seemed to be about something different.
>> 
>>> This may be old information now, but generally I don't see why
>>> imap.el should send poor protocol output to all servers just
>>> because Exchange is broken.
>> 
>> [It does have workarounds for various other servers, not that I want to
>> defend Exchange in any way.  Exchange 2007 is doing horrible things like
>> messing with MIME parts, which I don't think the previous version did,
>> and I wish I could avoid it.]
>> 
>> Anyhow, per the comment in the patch, 

> +  ;; FIXME:  Maybe it would be cleaner to have a flag to not signal
> +  ;; the error (which otherwise gives a message), and test
> +  ;; `imap-failed-tags'.  Also, Other IMAP clients use other forms of
> +  ;; request which work with Exchange, e.g. Claws does "UID FETCH 1:*
> +  ;; (UID)" rather than "FETCH UID 1,*".  Is there a good reason not
> +  ;; to do the same?

>> is there a good reason -- other than simplicity? -- to use FETCH
>> rather than UID like other clients?  I'm speaking mostly in
>> ignorance of IMAP...

> Comments?  (Simon?)

>> 2009-01-17  Dave Love  <fx@gnu.org>
>> 
>> * imap.el (imap-fetch-safe): Bind debug-on-error.
>> (imap-debug): Add imap-fetch-safe.

> Installed.

> Bjorn (and others), does the auto-detection work for you with the
> current version from CVS?

I just did a "cvs update" in trunk.  First off, I get the following
message

byte-code: Error in ~/.gnus: Please use `smtpmail.el' bundled with Emacs.

as smtpmail.el got restored in the contrib directory when I did the cvs
update.

But other than that, Gnus now starts up fine with no need to set
imap-enable-exchange-bug-workaround to t.

Thanks,

Bjorn.

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Dave Goldberg]

>>>>> On Sun, 01 Feb 2009 07:43:32 -0800, Bjorn Solberg <bjorn_ding2@hekneby.org> said:

> But other than that, Gnus now starts up fine with no need to set
> imap-enable-exchange-bug-workaround to t.

Unfortunately I cannot report the same success.  I use XEmacs:

"XEmacs 21.4 (patch 21) \"Educational Television\" [Lucid] (x86_64-unknown-linux, Mule) of Tue Jan  6 2009 on blackbird"

and if I don't set imap-enable-exchange-bug-workaround to t, I get
this backtrace:

Debugger entered--Lisp error: (error "Internal error, tag 13 status BAD code nil text The specified message set is invalid.")
  signal(error ("Internal error, tag 13 status BAD code nil text The specified message set is invalid."))
  cerror("Internal error, tag %s status %s code %s text %s" 13 BAD nil "The specified message set is invalid.")
  apply(cerror "Internal error, tag %s status %s code %s text %s" (13 BAD nil "The specified message set is invalid."))
  error("Internal error, tag %s status %s code %s text %s" 13 BAD nil "The specified message set is invalid.")
  imap-parse-response()
  imap-arrival-filter(#<process "imap<1>" pid 10620 state:run> "ecified message set is invalid.
\n")
  accept-process-output(#<process "imap<1>" pid 10620 state:run> 0 100)
  imap-wait-for-tag(13 nil)
  imap-send-command-wait("FETCH 1,* UID")
  imap-fetch("1,*" "UID" nil nouidfetch nil)
  byte-code("..." [nouidfetch receive props imap-enable-exchange-bug-workaround debug-on-error uids t imap-fetch buffer] 6)
  imap-fetch-safe(("1,*" . "1,*:*") "UID" nil nouidfetch)
  nnimap-find-minmax-uid("INBOX" examine)
  nnimap-retrieve-groups(("INBOX" "Calendar" "Sent Items" "INBOX/Miscellaneous" "INBOX/MessageArchive" "Contacts") "imc")
  gnus-retrieve-groups(("INBOX" "Calendar" "Sent Items" "INBOX/Miscellaneous" "INBOX/MessageArchive" "Contacts") (nnimap "imc" (nnimap-address "imc.mitre.org") (nnimap-stream starttls) (nnimap-server-port 143) (nnimap-need-unselect-to-notice-new-mail t) (nnimap-nov-is-evil t)))
  gnus-read-active-file-2(("INBOX" "Calendar" "Sent Items" "INBOX/Miscellaneous" "INBOX/MessageArchive" "Contacts") (nnimap "imc" (nnimap-address "imc.mitre.org") (nnimap-stream starttls) (nnimap-server-port 143) (nnimap-need-unselect-to-notice-new-mail t) (nnimap-nov-is-evil t)))
  gnus-read-active-file-1((nnimap "imc" (nnimap-address "imc.mitre.org") (nnimap-stream starttls) (nnimap-server-port 143) (nnimap-need-unselect-to-notice-new-mail t) (nnimap-nov-is-evil t)) nil)
  gnus-read-active-file(nil nil)
  gnus-setup-news(nil nil nil)
  byte-code("..." [gnus-slave gnus-current-startup-file gnus-startup-file did-connect level dont-connect gnus-group-quit gnus-run-hooks gnus-startup-hook gnus-make-newsrc-file gnus-dribble-read-file gnus-request-create-group "queue" (nndraft "") "drafts" (nndraft "") gnus-setup-news nil gnus-setup-news-hook gnus-start-draft-setup gnus-group-list-groups gnus-group-first-unread-group gnus-configure-windows group gnus-group-set-mode-line gnus-started-hook gnus-use-dribble-file gnus-agent] 4)
  gnus-1(nil nil nil)
  gnus(nil)
  call-interactively(gnus)
  command-execute(gnus t)
  execute-extended-command(nil)
  call-interactively(execute-extended-command)



-- 
Dave Goldberg
david.goldberg6@verizon.net

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Reiner Steib]

On Mon, Feb 02 2009, Dave Goldberg wrote:

> Unfortunately I cannot report the same success.  I use XEmacs:
>
> "XEmacs 21.4 (patch 21) \"Educational Television\" [Lucid] (x86_64-unknown-linux, Mule) of Tue Jan  6 2009 on blackbird"

Does anyone have an idea what is different in XEmacs?

Here's the relevant code:

(defun imap-fetch-safe (uids props &optional receive nouidfetch buffer)
  "Like `imap-fetch', but DTRT with Exchange 2007 bug.
However, UIDS here is a cons, where the car is the canonical form
of the UIDS specification, and the cdr is the one which works with
Exchange 2007 or, potentially, other buggy servers.
See `imap-enable-exchange-bug-workaround'."
  ;; The first time we get here for a given, we'll try the canonical
  ;; form.  If we get the known error from the buggy server, set the
  ;; flag buffer-locally (to account for connections to multiple
  ;; servers), then re-try with the alternative UIDS spec.  We don't
  ;; unconditionally use the alternative form, since the
  ;; currently-used alternatives are seriously inefficient with some
  ;; servers (although they are valid).
  ;;
  ;; FIXME:  Maybe it would be cleaner to have a flag to not signal
  ;; the error (which otherwise gives a message), and test
  ;; `imap-failed-tags'.  Also, Other IMAP clients use other forms of
  ;; request which work with Exchange, e.g. Claws does "UID FETCH 1:*
  ;; (UID)" rather than "FETCH UID 1,*".  Is there a good reason not
  ;; to do the same?
  (condition-case data
      ;; Binding `debug-on-error' allows us to get the error from
      ;; `imap-parse-response' -- it's normally caught by Emacs around
      ;; execution of a process filter.
      (let ((debug-on-error t))
	(imap-fetch (if imap-enable-exchange-bug-workaround
			(cdr uids)
		      (car uids))
		    props receive nouidfetch buffer))
    (error
     (if (and (not imap-enable-exchange-bug-workaround)
	      ;; This is the Exchange 2007 response.  It may be more
	      ;; robust just to check for a BAD response to the
	      ;; attempted fetch.
	      (string-match "The specified message set is invalid"
			    (cadr data)))
	 (with-current-buffer (or buffer (current-buffer))
	   (set (make-local-variable 'imap-enable-exchange-bug-workaround)
		t)
	   (imap-fetch (cdr uids) props receive nouidfetch))
       (signal (car data) (cdr data))))))

> and if I don't set imap-enable-exchange-bug-workaround to t, I get
> this backtrace:
>
> Debugger entered--Lisp error: (error "Internal error, tag 13 status BAD code nil text The specified message set is invalid.")
>   signal(error ("Internal error, tag 13 status BAD code nil text The specified message set is invalid."))
>   cerror("Internal error, tag %s status %s code %s text %s" 13 BAD nil "The specified message set is invalid.")
>   apply(cerror "Internal error, tag %s status %s code %s text %s" (13 BAD nil "The specified message set is invalid."))
>   error("Internal error, tag %s status %s code %s text %s" 13 BAD nil "The specified message set is invalid.")
>   imap-parse-response()
>   imap-arrival-filter(#<process "imap<1>" pid 10620 state:run> "ecified message set is invalid.
> \n")
>   accept-process-output(#<process "imap<1>" pid 10620 state:run> 0 100)
>   imap-wait-for-tag(13 nil)
>   imap-send-command-wait("FETCH 1,* UID")
>   imap-fetch("1,*" "UID" nil nouidfetch nil)
>   byte-code("..." [nouidfetch receive props imap-enable-exchange-bug-workaround debug-on-error uids t imap-fetch buffer] 6)
>   imap-fetch-safe(("1,*" . "1,*:*") "UID" nil nouidfetch)
>   nnimap-find-minmax-uid("INBOX" examine)
>   nnimap-retrieve-groups(("INBOX" "Calendar" "Sent Items" "INBOX/Miscellaneous" "INBOX/MessageArchive" "Contacts") "imc")
>   gnus-retrieve-groups(("INBOX" "Calendar" "Sent Items" "INBOX/Miscellaneous" "INBOX/MessageArchive" "Contacts") (nnimap "imc" (nnimap-address "imc.mitre.org") (nnimap-stream starttls) (nnimap-server-port 143) (nnimap-need-unselect-to-notice-new-mail t) (nnimap-nov-is-evil t)))
>   gnus-read-active-file-2(("INBOX" "Calendar" "Sent Items" "INBOX/Miscellaneous" "INBOX/MessageArchive" "Contacts") (nnimap "imc" (nnimap-address "imc.mitre.org") (nnimap-stream starttls) (nnimap-server-port 143) (nnimap-need-unselect-to-notice-new-mail t) (nnimap-nov-is-evil t)))
>   gnus-read-active-file-1((nnimap "imc" (nnimap-address "imc.mitre.org") (nnimap-stream starttls) (nnimap-server-port 143) (nnimap-need-unselect-to-notice-new-mail t) (nnimap-nov-is-evil t)) nil)
>   gnus-read-active-file(nil nil)
>   gnus-setup-news(nil nil nil)
>   byte-code("..." [gnus-slave gnus-current-startup-file gnus-startup-file did-connect level dont-connect gnus-group-quit gnus-run-hooks gnus-startup-hook gnus-make-newsrc-file gnus-dribble-read-file gnus-request-create-group "queue" (nndraft "") "drafts" (nndraft "") gnus-setup-news nil gnus-setup-news-hook gnus-start-draft-setup gnus-group-list-groups gnus-group-first-unread-group gnus-configure-windows group gnus-group-set-mode-line gnus-started-hook gnus-use-dribble-file gnus-agent] 4)
>   gnus-1(nil nil nil)
>   gnus(nil)
>   call-interactively(gnus)
>   command-execute(gnus t)
>   execute-extended-command(nil)
>   call-interactively(execute-extended-command)

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Stephen J. Turnbull]

Reiner Steib writes:
 > On Mon, Feb 02 2009, Dave Goldberg wrote:
 > 
 > > Unfortunately I cannot report the same success.  I use XEmacs:
 > >
 > > "XEmacs 21.4 (patch 21) \"Educational Television\" [Lucid] (x86_64-unknown-linux, Mule) of Tue Jan  6 2009 on blackbird"
 > 
 > Does anyone have an idea what is different in XEmacs?

I don't know the implementation of Emacs so I'm just guessing, but I
think the difference may be that XEmacs provides a way to debug the
filter function.  You asked for a debugger:

 >       (let ((debug-on-error t))

and you got it:

 > > Debugger entered--Lisp error: (error "Internal error, tag 13 status BAD code nil text The specified message set is invalid.")

(BTW, I really wish you wouldn't use "Internal error" for errors
signaled by application code because some external program is being
persnickety....)

I've never encountered this requirement before, but you could try
binding `debug-on-error' to a list not including the error thrown by
the process filter, or binding `debug-ignored-errors' to a list
including the error thrown by the process filter, whichever is less
likely to give (GNU) Emacs an upset stomach.

I'm not sure that I consider debug-on-error's documentation (which
says that the debugger won't be run if a condition-case handler would
catch the error) to be lying, since process filters are conceptually
run asynchronously (ie, by the event loop), not within the scope of
the condition-case.

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Dave Love]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> On Mon, Feb 02 2009, Dave Goldberg wrote:
>
>> Unfortunately I cannot report the same success.  I use XEmacs:
>>
>> "XEmacs 21.4 (patch 21) \"Educational Television\" [Lucid] (x86_64-unknown-linux, Mule) of Tue Jan  6 2009 on blackbird"
>
> Does anyone have an idea what is different in XEmacs?

I'll leave it for an XEmacs user to fix, but this should revert to the
previous situation where you have to set the variable explicitly for
XEmacs.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 900 bytes --]

2009-02-04  Dave Love  <fx@gnu.org>

	* imap.el (imap-fetch-safe): Revert last change in XEmacs.

Index: imap.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/imap.el,v
retrieving revision 7.54
diff -u -r7.54 imap.el
--- imap.el	31 Jan 2009 15:27:39 -0000	7.54
+++ imap.el	8 Feb 2009 17:58:23 -0000
@@ -1815,8 +1817,9 @@
   (condition-case data
       ;; Binding `debug-on-error' allows us to get the error from
       ;; `imap-parse-response' -- it's normally caught by Emacs around
-      ;; execution of a process filter.
-      (let ((debug-on-error t))
+      ;; execution of a process filter.  FIXME:  That doesn't work in
+      ;; XEmacs -- you get a debugger session.
+      (let ((debug-on-error (if (featurep 'xemacs) debug-on-error t)))
 	(imap-fetch (if imap-enable-exchange-bug-workaround
 			(cdr uids)
 		      (car uids))

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Dave Goldberg]

>>>>> On Sun, 08 Feb 2009 18:00:54 +0000, Dave Love <fx@gnu.org> said:

> Reiner Steib <reinersteib+gmane@imap.cc> writes:

>> On Mon, Feb 02 2009, Dave Goldberg wrote:
>> 
>>> Unfortunately I cannot report the same success.  I use XEmacs:
>>> 
>>> "XEmacs 21.4 (patch 21) \"Educational Television\" [Lucid] (x86_64-unknown-linux, Mule) of Tue Jan  6 2009 on blackbird"
>> 
>> Does anyone have an idea what is different in XEmacs?

> I'll leave it for an XEmacs user to fix, but this should revert to the
> previous situation where you have to set the variable explicitly for
> XEmacs.

I apologize for the misunderstanding.  To get to that point there was
no need for a change.  I only had the reported problem when I
commented out my setting of imap-enable-exchange-bug-workaround in my
init.el.  With that explicitly set to t, I have no problems at all.

-- 
Dave Goldberg
david.goldberg6@verizon.net

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Reiner Steib]

On Sun, Feb 01 2009, Bjorn Solberg wrote:

> Reiner Steib writes:
>> Bjorn 

your From address bounced.

>> (and others), does the auto-detection work for you with the current
>> version from CVS?
>
> I just did a "cvs update" in trunk.  First off, I get the following
> message
>
> byte-code: Error in ~/.gnus: Please use `smtpmail.el' bundled with Emacs.
>
> as smtpmail.el got restored in the contrib directory when I did the cvs
> update.

,----[ contrib/README ]
| Please note that it is NOT good to just add this directory to
| `load-path': a number of files in this directory will become part of
| more recent Emacs versions, so that you might be running obsolete
| libraries with all kinds of ill effects
| (cf. `list-load-path-shadows').
| 
| The suggested method for installation is to copy those files that
| you need to a directory which is in `load-path'.
`----

> But other than that, Gnus now starts up fine with no need to set
> imap-enable-exchange-bug-workaround to t.

Good to hear.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Bjorn Solberg]

Reiner Steib writes:

> On Tue, Jan 13 2009, Dave Love wrote:
>> I'm afraid there's something wrong with my fix, but it seems to be
>> something like a race condition, and isn't consistently reproducible.

> Bjorn Solberg reported that the fix wont work for him too:
> See <news:m3bpud5mmy.fsf@famsolberg.com> on ding, or 
> http://thread.gmane.org/gmane.emacs.gnus.general/68106/focus=68138
> where he posted the *imap-debug*" buffer.

Note that it's only upon startup I am seeing trouble.  Once Gnus has
started and displays the group list, I can

(setq imap-enable-exchange-bug-workaround nil)

and everything still works fine, albeit slow.  Fetching new mail and
using nnimap-split-inbox '("INBOX") and nnimap-split-rule processes
about 2s/header or 1/2 header/sec.  Thunderbird seems to open folders
and fetch headers from Exchange-2007 faster than Gnus also.  Using Gnus
and IMAP against GMail is fast though.

Bjorn.

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Bjorn Solberg]

Bjorn Solberg writes:

> Reiner Steib writes:
>> On Tue, Jan 13 2009, Dave Love wrote:
>>> I'm afraid there's something wrong with my fix, but it seems to be
>>> something like a race condition, and isn't consistently reproducible.

>> Bjorn Solberg reported that the fix wont work for him too:
>> See <news:m3bpud5mmy.fsf@famsolberg.com> on ding, or 
>> http://thread.gmane.org/gmane.emacs.gnus.general/68106/focus=68138
>> where he posted the *imap-debug*" buffer.

> Note that it's only upon startup I am seeing trouble.  Once Gnus has
> started and displays the group list, I can

> (setq imap-enable-exchange-bug-workaround nil)

> and everything still works fine, albeit slow.
[...]

I take that back.  The Gcc: still causes 

The specified message set is invalid.
Couldn't store article in group xxx: The specified message set is invalid.

to be displayed in the minibuffer when I send the message.  The message
still makes it to the folder, though, so the message is bogus since it
could store the article after all.  Having

  (setq imap-enable-exchange-bug-workaround t)

gives no such message in the minibuffer.

Re: IMAP and Exchange 2007 - imap-fetch-safe

[Dave Love]

Bjorn Solberg <bjorn_ding2@hekneby.org> writes:

> and everything still works fine, albeit slow.  Fetching new mail and
> using nnimap-split-inbox '("INBOX") and nnimap-split-rule processes
> about 2s/header or 1/2 header/sec.

It's not obviously slow for me (on a fast machine, these days), but I
haven't tried to time it against, say, Evolution.  I think there is too
much consing going on, but not necessarily in the IMAP code.  M-x
elp-instrument-package should reveal any bottleneck.

Re: FIXMEs in imap.el and nnimap.el

[Dave Love]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> Would someone take a look at the FIXMEs, please?

Obviously I could fix the customization of nnimap-split-rule, but is the
omitted form meant to be supported?

Re: FIXMEs in imap.el and nnimap.el

[Dave Love]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> Thanks, I've committed them with minor editing (sorry that this will
> cause merge conflicts for you):

For what it's worth, as you changed it generally, is there some
convention always to capitalize `FIXME'?

Conventions (was: FIXMEs in imap.el and nnimap.el)

[Reiner Steib]

On Mon, Jan 05 2009, Dave Love wrote:

> For what it's worth, as you changed it generally, is there some
> convention always to capitalize `FIXME'?

I dunno if there's an Emacs convention.  In the current Emacs trunk,
we have:

FIXME: 576
Fixme: 132
fixme: 17

At least it's my personal preference. :-)

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Conventions

[Tassilo Horn]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

Hi!

>> For what it's worth, as you changed it generally, is there some
>> convention always to capitalize `FIXME'?
>
> I dunno if there's an Emacs convention.  In the current Emacs trunk,
> we have:
>
> FIXME: 576
> Fixme: 132
> fixme: 17
>
> At least it's my personal preference. :-)

Ditto.  And it distinguishes better from usual comments, especially if
you use something like:

--8<---------------cut here---------------start------------->8---
(defvar th-font-lock-modes
  '(c-mode emacs-lisp-mode lisp-mode ruby-mode haskell-mode
           latex-mode texinfo-mode greql-mode greqlscript-mode
           lua-mode))

(dolist (mode th-font-lock-modes)
  (font-lock-add-keywords mode '(("\\<\\(FIXME\\|TODO\\)"
                                  1 'font-lock-warning-face t))))
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo