bug#22046: [PATCH] Improve version-to-list parsing

[Alex Dunn]

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).

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")))


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?