* bug#22046: [PATCH] Improve version-to-list parsing
@ 2015-11-28 22:26 Alex Dunn
2015-11-30 0:09 ` Alex Dunn
0 siblings, 1 reply; 10+ messages in thread
From: Alex Dunn @ 2015-11-28 22:26 UTC (permalink / raw)
To: 22046
[-- Attachment #1: Type: text/plain, Size: 882 bytes --]
This was prompted by an issue over at MELPA, where they were having
trouble packaging stable versions of erlang-mode due to Erlang’s odd
version-strings: https://github.com/milkypostman/melpa/issues/2553. So
with this patch, 'OTP-18.0.5' is valid and parsed as '(18 0 5).
Other changes in behavior:
- “.” can be used as a priority separator. This seemed appropriate
since Emacs allows changing of `version-separator' to strings other
than “.”, and in such cases “.” might be used to mark priority.
- The docstring said “.5” was invalid, when it actually was. I’ve made it
explicitly valid, and added tests for it.
- The docstring said “22.8X3” was invalid, when it actually was; it got
parsed as '(22 8 24 3). I’ve made it really invalid.
- I’ve made strings like “alpha3.2” valid, and parsed as '(3 2 -3).
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 14424 bytes --]
From 497052685f0b4aa4d2f0de2eb30f7beaf62c97c5 Mon Sep 17 00:00:00 2001
From: Alex Dunn <dunn.alex@gmail.com>
Date: Fri, 27 Nov 2015 15:39:24 -0800
Subject: [PATCH] Improve version-to-list parsing
* lisp/subr.el (version-regexp-alist): allow `.' as priority separator
* lisp/subr.el (version-to-list): parse versions like OTP-18.1.5 correctly
* lisp/subr.el (version-to-list): allow prefixed priority separators
* test/lisp/subr-tests.el (ert-test-version-parsing): parsing tests
---
lisp/subr.el | 100 +++++++++++++++++++++++++-------------
test/lisp/subr-tests.el | 125 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 191 insertions(+), 34 deletions(-)
diff --git a/lisp/subr.el b/lisp/subr.el
index ea926ae..74d6aa1 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4686,14 +4686,14 @@ version-separator
(defconst version-regexp-alist
- '(("^[-_+ ]?snapshot$" . -4)
+ '(("^[-\._+ ]?snapshot$" . -4)
;; treat "1.2.3-20050920" and "1.2-3" as snapshot releases
- ("^[-_+]$" . -4)
+ ("^[-\._+]$" . -4)
;; treat "1.2.3-CVS" as snapshot release
- ("^[-_+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)$" . -4)
- ("^[-_+ ]?alpha$" . -3)
- ("^[-_+ ]?beta$" . -2)
- ("^[-_+ ]?\\(pre\\|rc\\)$" . -1))
+ ("^[-\._+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)$" . -4)
+ ("^[-\._+ ]?alpha$" . -3)
+ ("^[-\._+ ]?beta$" . -2)
+ ("^[-\._+ ]?\\(pre\\|rc\\)$" . -1))
"Specify association between non-numeric version and its priority.
This association is used to handle version string like \"1.0pre2\",
@@ -4703,6 +4703,7 @@ version-regexp-alist
String Version Integer List Version
\"0.9snapshot\" (0 9 -4)
\"1.0-git\" (1 0 -4)
+ \"1.0.cvs\" (1 0 -4)
\"1.0pre2\" (1 0 -1 2)
\"1.0PRE2\" (1 0 -1 2)
\"22.8beta3\" (22 8 -2 3)
@@ -4742,41 +4743,68 @@ version-to-list
Examples of valid version syntax:
- 1.0pre2 1.0.7.5 22.8beta3 0.9alpha1 6.9.30Beta
+ 1.0pre2 1.0.7.5 22.8beta3 0.9alpha1 6.9.30Beta OTP-10.0.3 2.4.snapshot alpha3.2 .5
Examples of invalid version syntax:
- 1.0prepre2 1.0..7.5 22.8X3 alpha3.2 .5
+ 1.0prepre2 1.0..7.5 22.8X3
Examples of version conversion:
Version String Version as a List of Integers
- \"1.0.7.5\" (1 0 7 5)
- \"1.0pre2\" (1 0 -1 2)
- \"1.0PRE2\" (1 0 -1 2)
- \"22.8beta3\" (22 8 -2 3)
- \"22.8Beta3\" (22 8 -2 3)
- \"0.9alpha1\" (0 9 -3 1)
+ \".5\" (0 5)
+ \"0.9 alpha\" (0 9 -3)
\"0.9AlphA1\" (0 9 -3 1)
- \"0.9alpha\" (0 9 -3)
+ \"0.9alpha1\" (0 9 -3 1)
\"0.9snapshot\" (0 9 -4)
\"1.0-git\" (1 0 -4)
+ \"1.0.7.5\" (1 0 7 5)
+ \"1.0.cvs\" (1 0 -4)
+ \"1.0PRE2\" (1 0 -1 2)
+ \"1.0pre2\" (1 0 -1 2)
+ \"22.8 Beta3\" (22 8 -2 3)
+ \"22.8beta3\" (22 8 -2 3)
+ \"OTP-10.0.3\" (10 0 3)
+ \"alpha3.2\" (3 2 -3)
See documentation for `version-separator' and `version-regexp-alist'."
- (or (and (stringp ver) (> (length ver) 0))
- (error "Invalid version string: `%s'" ver))
- ;; Change .x.y to 0.x.y
- (if (and (>= (length ver) (length version-separator))
- (string-equal (substring ver 0 (length version-separator))
- version-separator))
+ (unless (stringp ver)
+ (error "Version must be a string"))
+ (let ((input ver) ; save the original version string for error messages
+ (i 0) ; our position in the version-string
+ (case-fold-search t) ; ignore case in matching
+ (al version-regexp-alist)
+ lst s pref) ; lst: the list returned
+ ; s: the starting index of the substring currently being parsed
+ ; pref: the priority of a prefixed separator
+ ;; Change .x.y to 0.x.y
+ (if (and (>= (length ver) (length version-separator))
+ (string-equal (substring ver 0 (length version-separator))
+ version-separator))
(setq ver (concat "0" ver)))
- (save-match-data
- (let ((i 0)
- (case-fold-search t) ; ignore case in matching
- lst s al)
+ ;; Allow text separators at the beginning of the version-string,
+ ;; but strip any other non-numeric prefix (see Erlang and their
+ ;; OTP-18.1.5 version-strings)
+ (when (and (string-match "^[^0-9]+" ver) (> (length ver) 0))
+ (setq i (match-end 0))
+ (while (and al (not (string-match (caar al) (substring ver 0 i))))
+ (setq al (cdr al)))
+ (if al
+ ;; If the non-numeric beginning of the version-string is a
+ ;; separator, set it to pref, to be added to lst at the end
+ (setq pref (cdar al))
+ ;; Else trim the version string up to the first number and reset i
+ (setq ver (substring ver i)
+ i 0)))
+ ;; Die here if the version-string is now empty
+ (if (= (length ver) 0)
+ (error "Invalid version string: `%s'" input))
+ (save-match-data
+ ;; Parse the version-string up to a separator until there are none left
(while (and (setq s (string-match "[0-9]+" ver i))
(= s i))
- ;; handle numeric part
+ ;; Add the numeric part to the beginning of the version list;
+ ;; lst gets reversed at the end
(setq lst (cons (string-to-number (substring ver i (match-end 0)))
lst)
i (match-end 0))
@@ -4790,17 +4818,21 @@ version-to-list
(setq al version-regexp-alist)
(while (and al (not (string-match (caar al) s)))
(setq al (cdr al)))
- (cond (al
+ ;; only allow alpha, beta, pre, etc. separators at the beginning
+ ;; of the version-string (handled above) or as a normal
+ ;; separator, but not as both ("beta0.9alpha1" is not valid)
+ (cond ((and al (null pref))
(push (cdar al) lst))
- ;; Convert 22.3a to 22.3.1, 22.3b to 22.3.2, etc.
- ((string-match "^[-_+ ]?\\([a-zA-Z]\\)$" s)
+ ;; Convert 22.3a to 22.3.1, 22.3b to 22.3.2, etc., but only if
+ ;; the letter is the end of the version-string, to avoid
+ ;; 22.8X3 being valid
+ ((and (string-match "^[-\._+ ]?\\([a-zA-Z]\\)$" s)
+ (= i (length ver)))
(push (- (aref (downcase (match-string 1 s)) 0) ?a -1)
lst))
- (t (error "Invalid version syntax: `%s'" ver))))))
- (if (null lst)
- (error "Invalid version syntax: `%s'" ver)
- (nreverse lst)))))
-
+ (t (error "Invalid version syntax: `%s'" input)))))))
+ (if pref (push pref lst))
+ (nreverse lst)))
(defun version-list-< (l1 l2)
"Return t if L1, a list specification of a version, is lower than L2.
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index ee8db59..605db91 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -103,5 +103,130 @@
(should (equal (macroexpand-all '(when a b c d))
'(if a (progn b c d)))))
+(ert-deftest subr-test-version-parsing ()
+ (should (equal (version-to-list ".5") '(0 5)))
+ (should (equal (version-to-list "0.9 alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0.9 snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9-alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0.9-snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9.snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9_snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0.9snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "1.0 git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0 pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0-git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0-pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0.1-a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1-f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.1.a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1.f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.1_a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1_f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.1a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.7.5") '(1 0 7 5)))
+ (should (equal (version-to-list "1.0.git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0.pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0_git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0_pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "22.8 beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8-beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8.beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8_beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "6.9.30 Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30-Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30.Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30_Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "OTP 18.1.5") '(18 1 5)))
+ (should (equal (version-to-list "OTP-18.1.5") '(18 1 5)))
+ (should (equal (version-to-list "OTP.18.1.5") '(18 1 5)))
+ (should (equal (version-to-list "OTP18.1.5") '(18 1 5)))
+ (should (equal (version-to-list "alpha0.9") '(0 9 -3)))
+
+ (should (equal
+ (error-message-string (should-error (version-to-list "")))
+ "Invalid version string: `'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "1.0..7.5")))
+ "Invalid version syntax: `1.0..7.5'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "1.0prepre2")))
+ "Invalid version syntax: `1.0prepre2'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "22.8X3")))
+ "Invalid version syntax: `22.8X3'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "beta22.8alpha")))
+ "Invalid version syntax: `beta22.8alpha'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "beta22.8alpha3")))
+ "Invalid version syntax: `beta22.8alpha3'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "honk")))
+ "Invalid version string: `honk'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list 9)))
+ "Version must be a string"))
+
+ (let ((version-separator "_"))
+ (should (equal (version-to-list "_5") '(0 5)))
+ (should (equal (version-to-list "0_9 alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9 snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0_9-alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9-snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0_9.alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9.snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0_9alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "1_0 git") '(1 0 -4)))
+ (should (equal (version-to-list "1_0 pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1_0-git") '(1 0 -4)))
+ (should (equal (version-to-list "1_0.pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1_0_1-a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1-f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_1.a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1.f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_1_a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1_f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_1a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_7_5") '(1 0 7 5)))
+ (should (equal (version-to-list "1_0_git") '(1 0 -4)))
+ (should (equal (version-to-list "1_0pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "22_8 beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22_8-beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22_8.beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22_8beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "6_9_30 Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6_9_30-Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6_9_30.Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6_9_30Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "OTP 18_1_5") '(18 1 5)))
+ (should (equal (version-to-list "OTP-18_1_5") '(18 1 5)))
+ (should (equal (version-to-list "OTP.18_1_5") '(18 1 5)))
+ (should (equal (version-to-list "OTP18_1_5") '(18 1 5)))
+ (should (equal (version-to-list "alpha0_9") '(0 9 -3)))
+
+ (should (equal
+ (error-message-string (should-error (version-to-list "1_0__7_5")))
+ "Invalid version syntax: `1_0__7_5'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "1_0prepre2")))
+ "Invalid version syntax: `1_0prepre2'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "22.8X3")))
+ "Invalid version syntax: `22.8X3'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "beta22_8alpha")))
+ "Invalid version syntax: `beta22_8alpha'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "beta22_8alpha3")))
+ "Invalid version syntax: `beta22_8alpha3'"))))
+
(provide 'subr-tests)
;;; subr-tests.el ends here
--
2.6.3
[-- Attachment #3: ChangeLog --]
[-- Type: text/plain, Size: 402 bytes --]
2015-11-28 Alex Dunn <dunn.alex@gmail.com>
Improve version-to-list parsing
* lisp/subr.el (version-regexp-alist): allow `.' as priority separator
* lisp/subr.el (version-to-list): parse versions like OTP-18.1.5 correctly
* lisp/subr.el (version-to-list): allow prefixed priority separators
* test/lisp/subr-tests.el (ert-test-version-parsing): parsing tests
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#22046: [PATCH] Improve version-to-list parsing
2015-11-28 22:26 bug#22046: [PATCH] Improve version-to-list parsing Alex Dunn
@ 2015-11-30 0:09 ` Alex Dunn
2015-11-30 3:34 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Alex Dunn @ 2015-11-30 0:09 UTC (permalink / raw)
To: 22046
Woops. “alpha0.9” is parsed as '(0 9 -3), but “alpha-0.9” is parsed as
'(0 9). Shouldn’t be hard to fix, though, if this behavior is desired.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#22046: [PATCH] Improve version-to-list parsing
2015-11-30 0:09 ` Alex Dunn
@ 2015-11-30 3:34 ` Eli Zaretskii
2015-11-30 3:54 ` Alex Dunn
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-11-30 3:34 UTC (permalink / raw)
To: Alex Dunn; +Cc: 22046
> From: Alex Dunn <dunn.alex@gmail.com>
> Date: Sun, 29 Nov 2015 16:09:12 -0800
>
>
> Woops. “alpha0.9” is parsed as '(0 9 -3), but “alpha-0.9” is parsed as
> '(0 9). Shouldn’t be hard to fix, though, if this behavior is desired.
Which part(s) of the string is/are the version in each case, in your
opinion?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#22046: [PATCH] Improve version-to-list parsing
2015-11-30 3:34 ` Eli Zaretskii
@ 2015-11-30 3:54 ` Alex Dunn
2015-11-30 7:59 ` Andreas Schwab
2015-11-30 15:50 ` Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: Alex Dunn @ 2015-11-30 3:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 22046
[-- Attachment #1: Type: text/plain, Size: 518 bytes --]
I’d say in both cases “0.9” is the version and /alpha-?/ is the priority
modifier, so if one is '(0 9 -3) then they both should be.
Two other options for dealing with these cases (while keeping
“OTP-18.0.5” -> '(18 0 5)) is to just strip the /alpha-?/ and parse
those strings as '(0 9) or flag them as invalid version-strings. My
ordered preferences are:
1. parse them both as '(0 9 -3)
2. treat them as invalid and throw an error
3. parse them as '(0 9)
FWIW, attaching the diff for (1).
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch on top of 0001-Improve-version-to-list-parsing.patch --]
[-- Type: text/x-patch, Size: 3044 bytes --]
diff --git a/lisp/subr.el b/lisp/subr.el
index 74d6aa1..dd3bac6 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4686,14 +4686,14 @@ version-separator
(defconst version-regexp-alist
- '(("^[-\._+ ]?snapshot$" . -4)
+ '(("^[-\._+ ]?snapshot" . -4)
;; treat "1.2.3-20050920" and "1.2-3" as snapshot releases
- ("^[-\._+]$" . -4)
+ ("^[-\._+]" . -4)
;; treat "1.2.3-CVS" as snapshot release
- ("^[-\._+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)$" . -4)
- ("^[-\._+ ]?alpha$" . -3)
- ("^[-\._+ ]?beta$" . -2)
- ("^[-\._+ ]?\\(pre\\|rc\\)$" . -1))
+ ("^[-\._+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)" . -4)
+ ("^[-\._+ ]?alpha" . -3)
+ ("^[-\._+ ]?beta" . -2)
+ ("^[-\._+ ]?\\(pre\\|rc\\)" . -1))
"Specify association between non-numeric version and its priority.
This association is used to handle version string like \"1.0pre2\",
@@ -4816,9 +4816,10 @@ version-to-list
;; handle alpha, beta, pre, etc. separator
(unless (string= s version-separator)
(setq al version-regexp-alist)
- (while (and al (not (string-match (caar al) s)))
+ ;; The regular expression needs to match the entire substring
+ (while (and al (not (string-match (concat (caar al) "$") s)))
(setq al (cdr al)))
- ;; only allow alpha, beta, pre, etc. separators at the beginning
+ ;; Only allow alpha, beta, pre, etc. separators at the beginning
;; of the version-string (handled above) or as a normal
;; separator, but not as both ("beta0.9alpha1" is not valid)
(cond ((and al (null pref))
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index 605db91..9d2365a 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -146,7 +146,9 @@
(should (equal (version-to-list "OTP-18.1.5") '(18 1 5)))
(should (equal (version-to-list "OTP.18.1.5") '(18 1 5)))
(should (equal (version-to-list "OTP18.1.5") '(18 1 5)))
+ (should (equal (version-to-list "alpha.0.9") '(0 9 -3)))
(should (equal (version-to-list "alpha0.9") '(0 9 -3)))
+ (should (equal (version-to-list "alpha_0.9") '(0 9 -3)))
(should (equal
(error-message-string (should-error (version-to-list "")))
@@ -210,7 +212,9 @@
(should (equal (version-to-list "OTP-18_1_5") '(18 1 5)))
(should (equal (version-to-list "OTP.18_1_5") '(18 1 5)))
(should (equal (version-to-list "OTP18_1_5") '(18 1 5)))
+ (should (equal (version-to-list "alpha.0_9") '(0 9 -3)))
(should (equal (version-to-list "alpha0_9") '(0 9 -3)))
+ (should (equal (version-to-list "alpha_0_9") '(0 9 -3)))
(should (equal
(error-message-string (should-error (version-to-list "1_0__7_5")))
[-- Attachment #3: Type: text/plain, Size: 384 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Alex Dunn <dunn.alex@gmail.com>
>> Date: Sun, 29 Nov 2015 16:09:12 -0800
>>
>>
>> Woops. “alpha0.9” is parsed as '(0 9 -3), but “alpha-0.9” is parsed as
>> '(0 9). Shouldn’t be hard to fix, though, if this behavior is desired.
>
> Which part(s) of the string is/are the version in each case, in your
> opinion?
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#22046: [PATCH] Improve version-to-list parsing
2015-11-30 3:54 ` Alex Dunn
@ 2015-11-30 7:59 ` Andreas Schwab
2015-11-30 15:50 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Andreas Schwab @ 2015-11-30 7:59 UTC (permalink / raw)
To: Alex Dunn; +Cc: 22046
Alex Dunn <dunn.alex@gmail.com> writes:
> diff --git a/lisp/subr.el b/lisp/subr.el
> index 74d6aa1..dd3bac6 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -4686,14 +4686,14 @@ version-separator
>
>
> (defconst version-regexp-alist
> - '(("^[-\._+ ]?snapshot$" . -4)
> + '(("^[-\._+ ]?snapshot" . -4)
> ;; treat "1.2.3-20050920" and "1.2-3" as snapshot releases
> - ("^[-\._+]$" . -4)
> + ("^[-\._+]" . -4)
> ;; treat "1.2.3-CVS" as snapshot release
> - ("^[-\._+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)$" . -4)
> - ("^[-\._+ ]?alpha$" . -3)
> - ("^[-\._+ ]?beta$" . -2)
> - ("^[-\._+ ]?\\(pre\\|rc\\)$" . -1))
> + ("^[-\._+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)" . -4)
> + ("^[-\._+ ]?alpha" . -3)
> + ("^[-\._+ ]?beta" . -2)
> + ("^[-\._+ ]?\\(pre\\|rc\\)" . -1))
> "Specify association between non-numeric version and its priority.
Is that part of the doc string no longer true?
REGEXP regexp used to match non-numeric part of a version string.
It should begin with the `^' anchor and end with a `$' to
prevent false hits. Letter-case is ignored while matching
REGEXP.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#22046: [PATCH] Improve version-to-list parsing
2015-11-30 3:54 ` Alex Dunn
2015-11-30 7:59 ` Andreas Schwab
@ 2015-11-30 15:50 ` Eli Zaretskii
2015-12-01 2:09 ` Alex Dunn
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-11-30 15:50 UTC (permalink / raw)
To: Alex Dunn; +Cc: 22046
> From: Alex Dunn <dunn.alex@gmail.com>
> Cc: 22046@debbugs.gnu.org
> Date: Sun, 29 Nov 2015 19:54:18 -0800
>
> I’d say in both cases “0.9” is the version and /alpha-?/ is the priority
> modifier, so if one is '(0 9 -3) then they both should be.
>
> Two other options for dealing with these cases (while keeping
> “OTP-18.0.5” -> '(18 0 5)) is to just strip the /alpha-?/ and parse
> those strings as '(0 9) or flag them as invalid version-strings. My
> ordered preferences are:
>
> 1. parse them both as '(0 9 -3)
> 2. treat them as invalid and throw an error
> 3. parse them as '(0 9)
The original code indeed signals an error with both "alpha0.9" and
"alpha-0.9", as expected.
But I've just realized that this is a followup to a previous patch, so
let me step back and respond to that.
> This was prompted by an issue over at MELPA, where they were having
> trouble packaging stable versions of erlang-mode due to Erlang’s odd
> version-strings: https://github.com/milkypostman/melpa/issues/2553. So
> with this patch, 'OTP-18.0.5' is valid and parsed as '(18 0 5).
Sorry, I don't understand the issue; can you clarify? "OTP-18.0.5" is
not a valid version string, you are supposed to submit just the
"18.0.5" part to the Emacs version-handling facilities. Why isn't
that being done here, or why cannot it be done? Especially since the
changes you propose effectively ignore the "OTP-" part anyway, as they
indeed should: AFAIU, "OTP" has nothing to do with versioning.
Treating "SOMETHING-1.2.3" as a valid version string changes the rules
significantly, and IMO opens a Pandora box, as we suddenly need to be
able to recognize/allow words that have nothing to do with versioning,
as opposed to a few words (alpha, beta, CVS, etc.) that do. I don't
think we should go that way without a very good reason and some
important use cases.
> - The docstring said “22.8X3” was invalid, when it actually was; it got
> parsed as '(22 8 24 3). I’ve made it really invalid.
This change in behavior is definitely worth making, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#22046: [PATCH] Improve version-to-list parsing
2015-11-30 15:50 ` Eli Zaretskii
@ 2015-12-01 2:09 ` Alex Dunn
2015-12-01 3:38 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Alex Dunn @ 2015-12-01 2:09 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 22046
The recipes MELPA uses for building packages are usually just the name,
the repository, and the method of fetching it (git, svn, etc). To
figure out if there’s a stable version of the package, MELPA parses the
repository’s tags; if the tags aren’t valid version-strings (according
to version-to-list) it assumes there isn’t a stable version available
and packages it as “HEAD-only”.
So MELPA is at the mercy of upstream developers’ tagging practices, and
sometimes they do things like “OTP-18.0.5”: https://github.com/erlang/otp/tags
Another solution to this particular problem is for MELPA to allow their
recipes to specify a custom version schema; but my thought was that
making version-to-list more flexible was a good thing. Parsing git tags
seems like a common enough use-case that it might be nice to have this.
But it’s true that with this change some very long strings will be
parsed as valid. This returns '(0 9), which is sort of ridiculous:
(version-to-list "It’s true that with this change some very long strings will be parsed as valid: 0.9")
But I guess I’m not sure what the danger is in letting that happen. Is
version-to-list often used to parse arbitrary strings, where false
positives would cause problems?
Thanks,
Alex
Eli Zaretskii <eliz@gnu.org> writes:
> But I've just realized that this is a followup to a previous patch, so
> let me step back and respond to that.
>
>> This was prompted by an issue over at MELPA, where they were having
>> trouble packaging stable versions of erlang-mode due to Erlang’s odd
>> version-strings: https://github.com/milkypostman/melpa/issues/2553. So
>> with this patch, 'OTP-18.0.5' is valid and parsed as '(18 0 5).
>
> Sorry, I don't understand the issue; can you clarify? "OTP-18.0.5" is
> not a valid version string, you are supposed to submit just the
> "18.0.5" part to the Emacs version-handling facilities. Why isn't
> that being done here, or why cannot it be done? Especially since the
> changes you propose effectively ignore the "OTP-" part anyway, as they
> indeed should: AFAIU, "OTP" has nothing to do with versioning.
>
> Treating "SOMETHING-1.2.3" as a valid version string changes the rules
> significantly, and IMO opens a Pandora box, as we suddenly need to be
> able to recognize/allow words that have nothing to do with versioning,
> as opposed to a few words (alpha, beta, CVS, etc.) that do. I don't
> think we should go that way without a very good reason and some
> important use cases.
>
>> - The docstring said “22.8X3” was invalid, when it actually was; it got
>> parsed as '(22 8 24 3). I’ve made it really invalid.
>
> This change in behavior is definitely worth making, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#22046: [PATCH] Improve version-to-list parsing
2015-12-01 2:09 ` Alex Dunn
@ 2015-12-01 3:38 ` Eli Zaretskii
2015-12-02 4:14 ` Alex Dunn
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-12-01 3:38 UTC (permalink / raw)
To: Alex Dunn; +Cc: 22046
> From: Alex Dunn <dunn.alex@gmail.com>
> Cc: 22046@debbugs.gnu.org
> Date: Mon, 30 Nov 2015 18:09:03 -0800
>
> The recipes MELPA uses for building packages are usually just the name,
> the repository, and the method of fetching it (git, svn, etc). To
> figure out if there’s a stable version of the package, MELPA parses the
> repository’s tags; if the tags aren’t valid version-strings (according
> to version-to-list) it assumes there isn’t a stable version available
> and packages it as “HEAD-only”.
>
> So MELPA is at the mercy of upstream developers’ tagging practices, and
> sometimes they do things like “OTP-18.0.5”: https://github.com/erlang/otp/tags
>
> Another solution to this particular problem is for MELPA to allow their
> recipes to specify a custom version schema; but my thought was that
> making version-to-list more flexible was a good thing. Parsing git tags
> seems like a common enough use-case that it might be nice to have this.
Sounds to me as a MELPA-specific problem that should be solved there,
not in Emacs.
> But it’s true that with this change some very long strings will be
> parsed as valid. This returns '(0 9), which is sort of ridiculous:
>
> (version-to-list "It’s true that with this change some very long strings will be parsed as valid: 0.9")
>
> But I guess I’m not sure what the danger is in letting that happen. Is
> version-to-list often used to parse arbitrary strings, where false
> positives would cause problems?
I just don't see a reason for such a radical change in behavior of a
feature that has been very stable lately.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#22046: [PATCH] Improve version-to-list parsing
2015-12-01 3:38 ` Eli Zaretskii
@ 2015-12-02 4:14 ` Alex Dunn
2015-12-05 9:36 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Alex Dunn @ 2015-12-02 4:14 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 22046
[-- Attachment #1: Type: text/plain, Size: 71 bytes --]
OK, that’s fair. Attaching a new patch and changelog.
Thanks!
[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 451 bytes --]
2015-12-01 Alex Dunn <dunn.alex@gmail.com>
Improve version-to-list parsing
* lisp/subr.el (version-regexp-alist): allow "." as priority separator
* lisp/subr.el (version-to-list): more helpful error messages
* lisp/subr.el (version-to-list): ".5" is valid (update docstring)
* lisp/subr.el (version-to-list): make "22.8X3" invalid
* test/lisp/subr-tests.el (ert-test-version-parsing): parsing tests
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Improve-version-to-list-parsing.patch --]
[-- Type: text/x-patch, Size: 11909 bytes --]
From e3a5c8f2a4b1020f2381f352048db988ad039feb Mon Sep 17 00:00:00 2001
From: Alex Dunn <dunn.alex@gmail.com>
Date: Fri, 27 Nov 2015 15:39:24 -0800
Subject: [PATCH] Improve version-to-list parsing
* lisp/subr.el (version-regexp-alist): allow "." as priority separator
* lisp/subr.el (version-to-list): more helpful error messages
* lisp/subr.el (version-to-list): ".5" is valid (update docstring)
* lisp/subr.el (version-to-list): make "22.8X3" invalid
* test/lisp/subr-tests.el (ert-test-version-parsing): parsing tests
---
lisp/subr.el | 55 +++++++++++++-----------
test/lisp/subr-tests.el | 112 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 143 insertions(+), 24 deletions(-)
diff --git a/lisp/subr.el b/lisp/subr.el
index ea926ae..a72d925 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4686,14 +4686,14 @@ version-separator
(defconst version-regexp-alist
- '(("^[-_+ ]?snapshot$" . -4)
+ '(("^[-\._+ ]?snapshot$" . -4)
;; treat "1.2.3-20050920" and "1.2-3" as snapshot releases
- ("^[-_+]$" . -4)
+ ("^[-\._+]$" . -4)
;; treat "1.2.3-CVS" as snapshot release
- ("^[-_+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)$" . -4)
- ("^[-_+ ]?alpha$" . -3)
- ("^[-_+ ]?beta$" . -2)
- ("^[-_+ ]?\\(pre\\|rc\\)$" . -1))
+ ("^[-\._+ ]?\\(cvs\\|git\\|bzr\\|svn\\|hg\\|darcs\\)$" . -4)
+ ("^[-\._+ ]?alpha$" . -3)
+ ("^[-\._+ ]?beta$" . -2)
+ ("^[-\._+ ]?\\(pre\\|rc\\)$" . -1))
"Specify association between non-numeric version and its priority.
This association is used to handle version string like \"1.0pre2\",
@@ -4703,6 +4703,7 @@ version-regexp-alist
String Version Integer List Version
\"0.9snapshot\" (0 9 -4)
\"1.0-git\" (1 0 -4)
+ \"1.0.cvs\" (1 0 -4)
\"1.0pre2\" (1 0 -1 2)
\"1.0PRE2\" (1 0 -1 2)
\"22.8beta3\" (22 8 -2 3)
@@ -4742,41 +4743,47 @@ version-to-list
Examples of valid version syntax:
- 1.0pre2 1.0.7.5 22.8beta3 0.9alpha1 6.9.30Beta
+ 1.0pre2 1.0.7.5 22.8beta3 0.9alpha1 6.9.30Beta 2.4.snapshot .5
Examples of invalid version syntax:
- 1.0prepre2 1.0..7.5 22.8X3 alpha3.2 .5
+ 1.0prepre2 1.0..7.5 22.8X3 alpha3.2
Examples of version conversion:
Version String Version as a List of Integers
- \"1.0.7.5\" (1 0 7 5)
- \"1.0pre2\" (1 0 -1 2)
- \"1.0PRE2\" (1 0 -1 2)
- \"22.8beta3\" (22 8 -2 3)
- \"22.8Beta3\" (22 8 -2 3)
- \"0.9alpha1\" (0 9 -3 1)
+ \".5\" (0 5)
+ \"0.9 alpha\" (0 9 -3)
\"0.9AlphA1\" (0 9 -3 1)
- \"0.9alpha\" (0 9 -3)
\"0.9snapshot\" (0 9 -4)
\"1.0-git\" (1 0 -4)
+ \"1.0.7.5\" (1 0 7 5)
+ \"1.0.cvs\" (1 0 -4)
+ \"1.0PRE2\" (1 0 -1 2)
+ \"1.0pre2\" (1 0 -1 2)
+ \"22.8 Beta3\" (22 8 -2 3)
+ \"22.8beta3\" (22 8 -2 3)
See documentation for `version-separator' and `version-regexp-alist'."
- (or (and (stringp ver) (> (length ver) 0))
- (error "Invalid version string: `%s'" ver))
+ (unless (stringp ver)
+ (error "Version must be a string"))
;; Change .x.y to 0.x.y
(if (and (>= (length ver) (length version-separator))
(string-equal (substring ver 0 (length version-separator))
version-separator))
(setq ver (concat "0" ver)))
+ (unless (string-match-p "^[0-9]" ver)
+ (error "Invalid version syntax: `%s' (must start with a number)" ver))
+
(save-match-data
(let ((i 0)
(case-fold-search t) ; ignore case in matching
lst s al)
+ ;; Parse the version-string up to a separator until there are none left
(while (and (setq s (string-match "[0-9]+" ver i))
(= s i))
- ;; handle numeric part
+ ;; Add the numeric part to the beginning of the version list;
+ ;; lst gets reversed at the end
(setq lst (cons (string-to-number (substring ver i (match-end 0)))
lst)
i (match-end 0))
@@ -4792,15 +4799,15 @@ version-to-list
(setq al (cdr al)))
(cond (al
(push (cdar al) lst))
- ;; Convert 22.3a to 22.3.1, 22.3b to 22.3.2, etc.
- ((string-match "^[-_+ ]?\\([a-zA-Z]\\)$" s)
+ ;; Convert 22.3a to 22.3.1, 22.3b to 22.3.2, etc., but only if
+ ;; the letter is the end of the version-string, to avoid
+ ;; 22.8X3 being valid
+ ((and (string-match "^[-\._+ ]?\\([a-zA-Z]\\)$" s)
+ (= i (length ver)))
(push (- (aref (downcase (match-string 1 s)) 0) ?a -1)
lst))
(t (error "Invalid version syntax: `%s'" ver))))))
- (if (null lst)
- (error "Invalid version syntax: `%s'" ver)
- (nreverse lst)))))
-
+ (nreverse lst))))
(defun version-list-< (l1 l2)
"Return t if L1, a list specification of a version, is lower than L2.
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index ee8db59..3fcb7d3 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -103,5 +103,117 @@
(should (equal (macroexpand-all '(when a b c d))
'(if a (progn b c d)))))
+(ert-deftest subr-test-version-parsing ()
+ (should (equal (version-to-list ".5") '(0 5)))
+ (should (equal (version-to-list "0.9 alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0.9 snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9-alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0.9-snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9.snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9_snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0.9alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0.9snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "1.0 git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0 pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0-git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0-pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0.1-a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1-f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.1.a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1.f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.1_a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1_f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.1a") '(1 0 1 1)))
+ (should (equal (version-to-list "1.0.1f") '(1 0 1 6)))
+ (should (equal (version-to-list "1.0.7.5") '(1 0 7 5)))
+ (should (equal (version-to-list "1.0.git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0.pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0_git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0_pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1.0git") '(1 0 -4)))
+ (should (equal (version-to-list "1.0pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "22.8 beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8-beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8.beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8_beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22.8beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "6.9.30 Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30-Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30.Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6.9.30_Beta") '(6 9 30 -2)))
+
+ (should (equal
+ (error-message-string (should-error (version-to-list "OTP-18.1.5")))
+ "Invalid version syntax: `OTP-18.1.5' (must start with a number)"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "")))
+ "Invalid version syntax: `' (must start with a number)"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "1.0..7.5")))
+ "Invalid version syntax: `1.0..7.5'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "1.0prepre2")))
+ "Invalid version syntax: `1.0prepre2'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "22.8X3")))
+ "Invalid version syntax: `22.8X3'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "beta22.8alpha3")))
+ "Invalid version syntax: `beta22.8alpha3' (must start with a number)"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "honk")))
+ "Invalid version syntax: `honk' (must start with a number)"))
+ (should (equal
+ (error-message-string (should-error (version-to-list 9)))
+ "Version must be a string"))
+
+ (let ((version-separator "_"))
+ (should (equal (version-to-list "_5") '(0 5)))
+ (should (equal (version-to-list "0_9 alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9 snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0_9-alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9-snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0_9.alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9.snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "0_9alpha1") '(0 9 -3 1)))
+ (should (equal (version-to-list "0_9snapshot") '(0 9 -4)))
+ (should (equal (version-to-list "1_0 git") '(1 0 -4)))
+ (should (equal (version-to-list "1_0 pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1_0-git") '(1 0 -4)))
+ (should (equal (version-to-list "1_0.pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "1_0_1-a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1-f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_1.a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1.f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_1_a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1_f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_1a") '(1 0 1 1)))
+ (should (equal (version-to-list "1_0_1f") '(1 0 1 6)))
+ (should (equal (version-to-list "1_0_7_5") '(1 0 7 5)))
+ (should (equal (version-to-list "1_0_git") '(1 0 -4)))
+ (should (equal (version-to-list "1_0pre2") '(1 0 -1 2)))
+ (should (equal (version-to-list "22_8 beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22_8-beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22_8.beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "22_8beta3") '(22 8 -2 3)))
+ (should (equal (version-to-list "6_9_30 Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6_9_30-Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6_9_30.Beta") '(6 9 30 -2)))
+ (should (equal (version-to-list "6_9_30Beta") '(6 9 30 -2)))
+
+ (should (equal
+ (error-message-string (should-error (version-to-list "1_0__7_5")))
+ "Invalid version syntax: `1_0__7_5'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "1_0prepre2")))
+ "Invalid version syntax: `1_0prepre2'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "22.8X3")))
+ "Invalid version syntax: `22.8X3'"))
+ (should (equal
+ (error-message-string (should-error (version-to-list "beta22_8alpha3")))
+ "Invalid version syntax: `beta22_8alpha3' (must start with a number)"))))
+
(provide 'subr-tests)
;;; subr-tests.el ends here
--
2.6.3
[-- Attachment #4: Type: text/plain, Size: 719 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
> Sounds to me as a MELPA-specific problem that should be solved there,
> not in Emacs.
>
>> But it’s true that with this change some very long strings will be
>> parsed as valid. This returns '(0 9), which is sort of ridiculous:
>>
>> (version-to-list "It’s true that with this change some very long strings will be parsed as valid: 0.9")
>>
>> But I guess I’m not sure what the danger is in letting that happen. Is
>> version-to-list often used to parse arbitrary strings, where false
>> positives would cause problems?
>
> I just don't see a reason for such a radical change in behavior of a
> feature that has been very stable lately.
>
> Thanks.
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#22046: [PATCH] Improve version-to-list parsing
2015-12-02 4:14 ` Alex Dunn
@ 2015-12-05 9:36 ` Eli Zaretskii
0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2015-12-05 9:36 UTC (permalink / raw)
To: Alex Dunn; +Cc: 22046
> From: Alex Dunn <dunn.alex@gmail.com>
> Cc: 22046@debbugs.gnu.org
> Date: Tue, 01 Dec 2015 20:14:29 -0800
>
> OK, that’s fair. Attaching a new patch and changelog.
Thanks, pushed.
Please note that you've exhausted your share of code contributions we
can accept without legal paperwork. I hope you will continue your
contributions, and encourage you to start the paperwork now. If you
agree, I will send you the forms off-list.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-12-05 9:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-28 22:26 bug#22046: [PATCH] Improve version-to-list parsing Alex Dunn
2015-11-30 0:09 ` Alex Dunn
2015-11-30 3:34 ` Eli Zaretskii
2015-11-30 3:54 ` Alex Dunn
2015-11-30 7:59 ` Andreas Schwab
2015-11-30 15:50 ` Eli Zaretskii
2015-12-01 2:09 ` Alex Dunn
2015-12-01 3:38 ` Eli Zaretskii
2015-12-02 4:14 ` Alex Dunn
2015-12-05 9:36 ` Eli Zaretskii
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).