unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP
@ 2020-01-05  3:20 Paul Eggert
  2020-01-05  8:54 ` Andreas Schwab
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Eggert @ 2020-01-05  3:20 UTC (permalink / raw)
  To: 38938; +Cc: Paul Eggert

Since Emacs has bignums now, it can represent IMAP integers
directly rather than as numeric strings.
* lisp/gnus/gnus-cus.el (gnus-extra-group-parameters):
* lisp/gnus/nnimap.el (nnimap-transform-headers)
(nnimap-convert-partial-article, nnimap-request-list)
(nnimap-parse-flags, nnimap-transform-split-mail):
* lisp/net/imap.el (imap-parse-resp-text-code)
(imap-parse-status):
Use Lisp integers to represent IMAP integers.
---
 lisp/gnus/gnus-cus.el |  4 ++--
 lisp/gnus/nnimap.el   | 39 ++++++++++++++-------------------------
 lisp/net/imap.el      | 26 +++++++++++++++-----------
 3 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/lisp/gnus/gnus-cus.el b/lisp/gnus/gnus-cus.el
index f0c4d07ca9..2581e092f9 100644
--- a/lisp/gnus/gnus-cus.el
+++ b/lisp/gnus/gnus-cus.el
@@ -252,11 +252,11 @@ gnus-extra-topic-parameters
 DOC is a documentation string for the parameter.")
 
 (defconst gnus-extra-group-parameters
-  '((uidvalidity (string :tag "IMAP uidvalidity") "\
+  '((uidvalidity (integer :tag "IMAP uidvalidity") "\
 Server-assigned value attached to IMAP groups, used to maintain consistency.")
     (modseq (choice :tag "modseq"
 		    (const :tag "None" nil)
-		    (string :tag "Sequence number"))
+		    (integer :tag "Sequence number"))
 	    "Modification sequence number")
     (active (cons :tag "active" (integer :tag "min") (integer :tag "max"))
 	    "active")
diff --git a/lisp/gnus/nnimap.el b/lisp/gnus/nnimap.el
index c383e0146f..73c2f3174c 100644
--- a/lisp/gnus/nnimap.el
+++ b/lisp/gnus/nnimap.el
@@ -261,9 +261,8 @@ nnimap-transform-headers
 	  (insert (format "%S" (subst-char-in-string ?\n ?\s string))))
 	(beginning-of-line)
 	(setq article
-	      (and (re-search-forward "UID \\([0-9]+\\)" (line-end-position)
-				      t)
-		   (match-string 1)))
+	      (and (search-forward "UID " (line-end-position) t)
+		   (ignore-errors (read (current-buffer)))))
 	;; If we've already got headers for this article, or this
 	;; FETCH line doesn't provide headers for the article, skip
 	;; it.  See bug#35433.
@@ -278,10 +277,8 @@ nnimap-transform-headers
 	  (setq lines nil)
 	  (beginning-of-line)
 	  (setq size
-		(and (re-search-forward "RFC822.SIZE \\([0-9]+\\)"
-					(line-end-position)
-					t)
-		     (match-string 1)))
+		(and (search-forward "RFC822.SIZE " (line-end-position) t)
+		     (ignore-errors (read (current-buffer)))))
 	  (beginning-of-line)
 	  (when (search-forward "X-GM-LABELS" (line-end-position) t)
 	    (setq labels (ignore-errors (read (current-buffer)))))
@@ -299,10 +296,10 @@ nnimap-transform-headers
 			      (nth 9 structure)
 			    (nth 7 structure)))))
 	  (delete-region (line-beginning-position) (line-end-position))
-	  (insert (format "211 %s Article retrieved." article))
+	  (insert (format "211 %d Article retrieved." article))
 	  (forward-line 1)
 	  (when size
-	    (insert (format "Chars: %s\n" size)))
+	    (insert (format "Chars: %d\n" size)))
 	  (when lines
 	    (insert (format "Lines: %s\n" lines)))
 	  (when labels
@@ -771,7 +768,7 @@ nnimap-convert-partial-article
     (goto-char (+ (point) bytes))
     ;; Collect all the body parts.
     (while (looking-at ".*BODY\\[\\([.0-9]+\\)\\]")
-      (setq id (match-string 1)
+      (setq id (string-to-number (match-string 1))
 	    bytes (or (nnimap-get-length) 0))
       (beginning-of-line)
       (delete-region (point) (progn (forward-line 1) (point)))
@@ -1391,7 +1388,7 @@ nnimap-request-list
 		    (when (equal (cadr elem) "EXISTS")
 		      (setq exists (string-to-number (car elem)))))
 		  (when uidnext
-		    (setq highest (1- (string-to-number (car uidnext)))))
+		    (setq highest (1- (car uidnext))))
 		  (cond
 		   ((null highest)
 		    (insert (format "%S 0 1 y\n" group)))
@@ -1770,10 +1767,6 @@ nnimap-parse-flags
   ;; read it.
   (subst-char-in-region (point-min) (point-max)
 			?\\ ?% t)
-  ;; Remove any MODSEQ entries in the buffer, because they may contain
-  ;; numbers that are too large for 32-bit Emacsen.
-  (while (re-search-forward " MODSEQ ([0-9]+)" nil t)
-    (replace-match "" t t))
   (goto-char (point-min))
   (let (start end articles groups uidnext elems permanent-flags
 	      uidvalidity vanished highestmodseq)
@@ -1799,11 +1792,8 @@ nnimap-parse-flags
 			    (read (current-buffer))))
 		 (goto-char start)
 		 (setq uidvalidity
-		       (and (re-search-forward "UIDVALIDITY \\([0-9]+\\)"
-					       end t)
-			    ;; Store UIDVALIDITY as a string, as it's
-			    ;; too big for 32-bit Emacsen, usually.
-			    (match-string 1)))
+		       (and (search-forward "UIDVALIDITY " end t)
+			    (read (current-buffer))))
 		 (goto-char start)
 		 (setq vanished
 		       (and (eq flag-sequence 'qresync)
@@ -1812,9 +1802,8 @@ nnimap-parse-flags
 			    (match-string 1)))
 		 (goto-char start)
 		 (setq highestmodseq
-		       (and (re-search-forward "HIGHESTMODSEQ \\([0-9]+\\)"
-					       end t)
-			    (match-string 1)))
+		       (and (search-forward "HIGHESTMODSEQ " end t)
+			    (read (current-buffer))))
 		 (goto-char end)
 		 (forward-line -1))
 	       ;; The UID FETCH FLAGS was successful.
@@ -2223,12 +2212,12 @@ nnimap-transform-split-mail
 	  (delete-region (point) (progn (forward-line 1) (point)))
 	  (when (eobp)
 	    (cl-return)))
-	(setq article (match-string 1)
+	(setq article (string-to-number (match-string 1))
 	      bytes (nnimap-get-length))
 	(delete-region (line-beginning-position) (line-end-position))
 	;; Insert MMDF separator, and a way to remember what this
 	;; article UID is.
-	(insert (format "\^A\^A\^A\^A\n\nX-nnimap-article: %s" article))
+	(insert (format "\^A\^A\^A\^A\n\nX-nnimap-article: %d" article))
 	(forward-char (1+ bytes))
 	(setq bytes (nnimap-get-length))
 	(delete-region (line-beginning-position) (line-end-position))
diff --git a/lisp/net/imap.el b/lisp/net/imap.el
index aa10f0291f..b46a4c8728 100644
--- a/lisp/net/imap.el
+++ b/lisp/net/imap.el
@@ -108,7 +108,7 @@
 ;; => 166
 ;;
 ;; (imap-mailbox-get 'uidvalidity)
-;; => "908992622"
+;; => 908992622
 ;;
 ;; (imap-search "FLAGGED SINCE 18-DEC-98")
 ;; => (235 236)
@@ -2337,12 +2337,12 @@ imap-parse-resp-text-code
     (imap-forward)
     (cond ((search-forward "PERMANENTFLAGS " nil t)
 	   (imap-mailbox-put 'permanentflags (imap-parse-flag-list)))
-	  ((search-forward "UIDNEXT \\([0-9]+\\)" nil t)
-	   (imap-mailbox-put 'uidnext (match-string 1)))
+	  ((search-forward "UIDNEXT " nil t)
+	   (imap-mailbox-put 'uidnext (read (current-buffer))))
 	  ((search-forward "UNSEEN " nil t)
 	   (imap-mailbox-put 'first-unseen (read (current-buffer))))
-	  ((looking-at "UIDVALIDITY \\([0-9]+\\)")
-	   (imap-mailbox-put 'uidvalidity (match-string 1)))
+	  ((looking-at "UIDVALIDITY ")
+	   (imap-mailbox-put 'uidvalidity (read (current-buffer))))
 	  ((search-forward "READ-ONLY" nil t)
 	   (imap-mailbox-put 'read-only t))
 	  ((search-forward "NEWNAME " nil t)
@@ -2353,13 +2353,13 @@ imap-parse-resp-text-code
 	     (imap-mailbox-put 'newname newname oldname)))
 	  ((search-forward "TRYCREATE" nil t)
 	   (imap-mailbox-put 'trycreate t imap-current-target-mailbox))
-	  ((looking-at "APPENDUID \\([0-9]+\\) \\([0-9]+\\)")
+	  ((looking-at "APPENDUID ")
 	   (imap-mailbox-put 'appenduid
-			     (list (match-string 1)
-				   (string-to-number (match-string 2)))
+			     (list (read (current-buffer))
+				   (read (current-buffer)))
 			     imap-current-target-mailbox))
 	  ((looking-at "COPYUID \\([0-9]+\\) \\([0-9,:]+\\) \\([0-9,:]+\\)")
-	   (imap-mailbox-put 'copyuid (list (match-string 1)
+	   (imap-mailbox-put 'copyuid (list (string-to-number (match-string 1))
 					    (match-string 2)
 					    (match-string 3))
 			     imap-current-target-mailbox))
@@ -2521,11 +2521,15 @@ imap-parse-status
 		 (imap-mailbox-put 'recent (read (current-buffer)) mailbox))
 		((string= token "UIDNEXT")
 		 (and (looking-at "[0-9]+")
-		      (imap-mailbox-put 'uidnext (match-string 0) mailbox)
+		      (imap-mailbox-put 'uidnext
+					(string-to-number (match-string 0))
+					mailbox)
 		      (goto-char (match-end 0))))
 		((string= token "UIDVALIDITY")
 		 (and (looking-at "[0-9]+")
-		      (imap-mailbox-put 'uidvalidity (match-string 0) mailbox)
+		      (imap-mailbox-put 'uidvalidity
+					(string-to-number (match-string 0))
+					mailbox)
 		      (goto-char (match-end 0))))
 		((string= token "UNSEEN")
 		 (imap-mailbox-put 'unseen (read (current-buffer)) mailbox))
-- 
2.17.1






^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP
  2020-01-05  3:20 bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP Paul Eggert
@ 2020-01-05  8:54 ` Andreas Schwab
  2020-09-20 10:17   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2020-01-05  8:54 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 38938

On Jan 04 2020, Paul Eggert wrote:

> Since Emacs has bignums now, it can represent IMAP integers
> directly rather than as numeric strings.
> * lisp/gnus/gnus-cus.el (gnus-extra-group-parameters):
> * lisp/gnus/nnimap.el (nnimap-transform-headers)
> (nnimap-convert-partial-article, nnimap-request-list)
> (nnimap-parse-flags, nnimap-transform-split-mail):
> * lisp/net/imap.el (imap-parse-resp-text-code)
> (imap-parse-status):
> Use Lisp integers to represent IMAP integers.

This breaks backward and forward compatibility.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP
  2020-01-05  8:54 ` Andreas Schwab
@ 2020-09-20 10:17   ` Lars Ingebrigtsen
  2020-09-20 17:04     ` Andy Moreton
  2020-10-13 17:27     ` Paul Eggert
  0 siblings, 2 replies; 6+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-20 10:17 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 38938, Paul Eggert

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Jan 04 2020, Paul Eggert wrote:
>
>> Since Emacs has bignums now, it can represent IMAP integers
>> directly rather than as numeric strings.
>> * lisp/gnus/gnus-cus.el (gnus-extra-group-parameters):
>> * lisp/gnus/nnimap.el (nnimap-transform-headers)
>> (nnimap-convert-partial-article, nnimap-request-list)
>> (nnimap-parse-flags, nnimap-transform-split-mail):
>> * lisp/net/imap.el (imap-parse-resp-text-code)
>> (imap-parse-status):
>> Use Lisp integers to represent IMAP integers.
>
> This breaks backward and forward compatibility.

Yeah, so while it'd be better to use integers here, I don't think it's
workable, and I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP
  2020-09-20 10:17   ` Lars Ingebrigtsen
@ 2020-09-20 17:04     ` Andy Moreton
  2020-09-20 19:22       ` Lars Ingebrigtsen
  2020-10-13 17:27     ` Paul Eggert
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Moreton @ 2020-09-20 17:04 UTC (permalink / raw)
  To: 38938

On Sun 20 Sep 2020, Lars Ingebrigtsen wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> On Jan 04 2020, Paul Eggert wrote:
>>
>>> Since Emacs has bignums now, it can represent IMAP integers
>>> directly rather than as numeric strings.
>>> * lisp/gnus/gnus-cus.el (gnus-extra-group-parameters):
>>> * lisp/gnus/nnimap.el (nnimap-transform-headers)
>>> (nnimap-convert-partial-article, nnimap-request-list)
>>> (nnimap-parse-flags, nnimap-transform-split-mail):
>>> * lisp/net/imap.el (imap-parse-resp-text-code)
>>> (imap-parse-status):
>>> Use Lisp integers to represent IMAP integers.
>>
>> This breaks backward and forward compatibility.
>
> Yeah, so while it'd be better to use integers here, I don't think it's
> workable, and I'm closing this bug report.

It may be useful to update the comments in the existing code to ensure
maintainers who are not aware of this compatibility issue know not to
attempt a similar change in the future.

    AndyM






^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP
  2020-09-20 17:04     ` Andy Moreton
@ 2020-09-20 19:22       ` Lars Ingebrigtsen
  0 siblings, 0 replies; 6+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-20 19:22 UTC (permalink / raw)
  To: Andy Moreton; +Cc: 38938

Andy Moreton <andrewjmoreton@gmail.com> writes:

> It may be useful to update the comments in the existing code to ensure
> maintainers who are not aware of this compatibility issue know not to
> attempt a similar change in the future.

I don't quite know where to put a note like that where it would make a
difference -- "don't break the .newsrc.eld file" is pretty general and
the data structures are updated from all over the place...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP
  2020-09-20 10:17   ` Lars Ingebrigtsen
  2020-09-20 17:04     ` Andy Moreton
@ 2020-10-13 17:27     ` Paul Eggert
  1 sibling, 0 replies; 6+ messages in thread
From: Paul Eggert @ 2020-10-13 17:27 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Andreas Schwab; +Cc: 38938, Andy Moreton

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

On 9/20/20 3:17 AM, Lars Ingebrigtsen wrote:

> so while it'd be better to use integers here, I don't think it's
> workable

OK. Andy suggested updating comments and I did find one that needed that; 
perhaps more comments would be useful too, but one thing at a time.

Also, there's no longer a need to remove MODSEQs from the buffer since they 
parse OK now even when they are large integers.

So I installed the attached minor fixup instead of the original patch I proposed.

[-- Attachment #2: 0001-nnimap-MODSEQ-cleanup.patch --]
[-- Type: text/x-patch, Size: 1577 bytes --]

From 1274d0eed67470f9c3d001ff60d76eeefa6983b9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 13 Oct 2020 10:21:40 -0700
Subject: [PATCH] nnimap MODSEQ cleanup

* lisp/gnus/nnimap.el (nnimap-parse-flags):
Remove old hack that deletes MODSEQ entries in the buffer, as
Emacs now has bignums and so won't misparse MODSEQs (Bug#38938).
---
 lisp/gnus/nnimap.el | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/lisp/gnus/nnimap.el b/lisp/gnus/nnimap.el
index d797e893f5..8a88e0e6e6 100644
--- a/lisp/gnus/nnimap.el
+++ b/lisp/gnus/nnimap.el
@@ -1772,11 +1772,6 @@ nnimap-parse-flags
   ;; read it.
   (subst-char-in-region (point-min) (point-max)
 			?\\ ?% t)
-  ;; Remove any MODSEQ entries in the buffer, because they may contain
-  ;; numbers that are too large for 32-bit Emacsen.
-  (while (re-search-forward " MODSEQ ([0-9]+)" nil t)
-    (replace-match "" t t))
-  (goto-char (point-min))
   (let (start end articles groups uidnext elems permanent-flags
 	      uidvalidity vanished highestmodseq)
     (dolist (elem sequences)
@@ -1803,8 +1798,9 @@ nnimap-parse-flags
 		 (setq uidvalidity
 		       (and (re-search-forward "UIDVALIDITY \\([0-9]+\\)"
 					       end t)
-			    ;; Store UIDVALIDITY as a string, as it's
-			    ;; too big for 32-bit Emacsen, usually.
+			    ;; Store UIDVALIDITY as a string; before bignums,
+			    ;; it was usually too big for 32-bit Emacsen,
+			    ;; and we don't want to change the format now.
 			    (match-string 1)))
 		 (goto-char start)
 		 (setq vanished
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-13 17:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-05  3:20 bug#38938: [PROPOSED] Prefer Lisp integers to numeric strings in IMAP Paul Eggert
2020-01-05  8:54 ` Andreas Schwab
2020-09-20 10:17   ` Lars Ingebrigtsen
2020-09-20 17:04     ` Andy Moreton
2020-09-20 19:22       ` Lars Ingebrigtsen
2020-10-13 17:27     ` Paul Eggert

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).