Regexp error scan (March 26)

Regexp error scan (March 26)

[Mattias Engdegård]

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

This is the latest regexp error scan of Emacs source files, using relint 1.5 (in ELPA now, or when it updates), xr 1.9.
New checks has permitted it to discover more irregularities.

The log contains, at the end, checks found with an experimental version of xr that yielded too many false positives to be useful in general, but these errors seem to be genuine.

[-- Attachment #2: relint.log --]
[-- Type: application/octet-stream, Size: 13328 bytes --]

;; -*- compilation -*-
lisp/erc/erc.el:2526:16: Value from `regexp-quote' cannot be spliced into `[...]'
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ת' inside character alternative (pos 23)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   .......................^
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ק' inside character alternative (pos 24)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   ........................^
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ר' inside character alternative (pos 26)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   ..........................^
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ת' inside character alternative (pos 27)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   ...........................^
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ש' inside character alternative (pos 28)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   ............................^
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ת' inside character alternative (pos 30)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   ..............................^
lisp/mail/footnote.el:366:3: In footnote-hebrew-numeric-regex: Duplicated `ת' inside character alternative (pos 32)
  "[אבגדהוזחטיכלמנסעפצקרשתתקתרתשתתתתק']+"
   ................................^
lisp/mail/footnote.el:564:27: In call to looking-back: Repetition of repetition (pos 8)
  "\\[[0-9]+*]"
   .........^
lisp/mail/footnote.el:576:23: In call to looking-at: Repetition of repetition (pos 8)
  "\\[[0-9]+*]"
   .........^
lisp/mail/footnote.el:841:31: In call to looking-back: Repetition of repetition (pos 8)
  "\\[[0-9]+*]"
   .........^
lisp/progmodes/verilog-mode.el:2062:3: In verilog-directive-re: Duplicated alternative branch (pos 372)
  "\\<\\(`__FILE__\\|`__LINE\\|`begin_keywords\\|`celldefine\\|`default_nettype\\|`define\\|`else\\|`elsif\\|`end_keywords\\|`endcelldefine\\|`endif\\|`ifdef\\|`ifndef\\|`include\\|`line\\|`nounconnected_drive\\|`pragma\\|`resetall\\|`timescale\\|`unconnected_drive\\|`undef\\|`undefineall\\|`case\\|`default\\|`endfor\\|`endprotect\\|`endswitch\\|`endwhile\\|`for\\|`format\\|`if\\|`let\\|`protect\\|`switch\\|`timescale\\|`time_scale\\|`while\\)\\>"
   ........................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/progmodes/verilog-mode.el:2412:3: In verilog-assignment-operator-re: Duplicated alternative branch (pos 84)
  "\\(=\\|\\+=\\|-=\\|\\*=\\|/=\\|%=\\|&=\\||=\\|\\^=\\|<<=\\|>>=\\|<<<=\\|>>>=\\|<=\\|==\\|!=\\|===\\|!==\\|<=\\|>=\\|==\\?\\|!=\\?\\|<->\\|->\\|->>\\||->\\||=>\\|#-#\\|#=#\\|:=\\|:/\\)"
   ..........................................................................................................^
lisp/progmodes/verilog-mode.el:2430:3: In verilog-assignment-operation-re: Duplicated alternative branch (pos 94)
  "\\(^.*?\\)\\B\\(=\\|\\+=\\|-=\\|\\*=\\|/=\\|%=\\|&=\\||=\\|\\^=\\|<<=\\|>>=\\|<<<=\\|>>>=\\|<=\\|==\\|!=\\|===\\|!==\\|<=\\|>=\\|==\\?\\|!=\\?\\|<->\\|->\\|->>\\||->\\||=>\\|#-#\\|#=#\\|:=\\|:/\\)\\B"
   .......................................................................................................................^
lisp/progmodes/verilog-mode.el:2486:3: In verilog-auto-end-comment-lines-re: Duplicated alternative branch (pos 374)
  "\\(\\<\\(`__FILE__\\|`__LINE\\|`begin_keywords\\|`celldefine\\|`default_nettype\\|`define\\|`else\\|`elsif\\|`end_keywords\\|`endcelldefine\\|`endif\\|`ifdef\\|`ifndef\\|`include\\|`line\\|`nounconnected_drive\\|`pragma\\|`resetall\\|`timescale\\|`unconnected_drive\\|`undef\\|`undefineall\\|`case\\|`default\\|`endfor\\|`endprotect\\|`endswitch\\|`endwhile\\|`for\\|`format\\|`if\\|`let\\|`protect\\|`switch\\|`timescale\\|`time_scale\\|`while\\)\\>\\)\\|\\(\\<\\(begin\\|else\\|end\\|endcase\\|endclass\\|endclocking\\|endgroup\\|endfunction\\|endmodule\\|endprogram\\|endprimitive\\|endinterface\\|endpackage\\|endsequence\\|endproperty\\|endspecify\\|endtable\\|endtask\\|join\\|join_any\\|join_none\\|module\\|macromodule\\|primitive\\|interface\\|package\\)\\>\\)"
   ...........................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/progmodes/verilog-mode.el:2946:3: In verilog-preprocessor-re: Escaped non-special character `/' (pos 423)
  "\\(?:\\<\\(`__FILE__\\|`__LINE__\\|`celldefine\\|`else\\|`end_keywords\\|`endcelldefine\\|`endif\\|`nounconnected_drive\\|`resetall\\|`unconnected_drive\\|`undefineall\\)\\>\\)\\|\\(?:\\<\\(`elsif\\|`ifn?def\\|`undef\\|`default_nettype\\|`begin_keywords\\)\\>\\s-\\)\\|\\(?:\\<\\(`line\\)\\>\\s-+[0-9]+\\s-+\"[^\"]+\"\\s-+[012]\\)\\|\\(?:\\<\\(`include\\)\\>\\s-+\\(?:\"[^\"]+\"\\|<[^>]+>\\)\\)\\|\\(?:\\<\\(`pragma\\)\\>\\s-+.+$\\)\\|\\(?:\\<\\(`timescale\\)\\>\\s-+10\\{0,2\\}\\s-*[munpf]?s\\s-*\\/\\s-*10\\{0,2\\}\\s-*[munpf]?s\\)\\|\\(?:\\<\\(`define\\|`if\\)\\>\\s-+\\(?:.*?\\(?:\n.*\\)*?\\)\\(?:\n\\s-*\n\\|\\'\\)\\)\\|\\(?:\\<`\\w+\\>\\s-*(\\(?:.*?\\(?:\n.*\\)*?\\)\\(?:\n\\s-*\n\\|\\'\\)\\)"
   .................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/progmodes/verilog-mode.el:5562:57: In call to looking-at: Duplicated alternative branch (pos 377)
  "[ \t]*\\<\\(`__FILE__\\|`__LINE\\|`begin_keywords\\|`celldefine\\|`default_nettype\\|`define\\|`else\\|`elsif\\|`end_keywords\\|`endcelldefine\\|`endif\\|`ifdef\\|`ifndef\\|`include\\|`line\\|`nounconnected_drive\\|`pragma\\|`resetall\\|`timescale\\|`unconnected_drive\\|`undef\\|`undefineall\\|`case\\|`default\\|`endfor\\|`endprotect\\|`endswitch\\|`endwhile\\|`for\\|`format\\|`if\\|`let\\|`protect\\|`switch\\|`timescale\\|`time_scale\\|`while\\)\\>"
   ..............................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/progmodes/verilog-mode.el:6215:37: In call to looking-at: Duplicated alternative branch (pos 377)
  "[ \t]*\\<\\(`__FILE__\\|`__LINE\\|`begin_keywords\\|`celldefine\\|`default_nettype\\|`define\\|`else\\|`elsif\\|`end_keywords\\|`endcelldefine\\|`endif\\|`ifdef\\|`ifndef\\|`include\\|`line\\|`nounconnected_drive\\|`pragma\\|`resetall\\|`timescale\\|`unconnected_drive\\|`undef\\|`undefineall\\|`case\\|`default\\|`endfor\\|`endprotect\\|`endswitch\\|`endwhile\\|`for\\|`format\\|`if\\|`let\\|`protect\\|`switch\\|`timescale\\|`time_scale\\|`while\\)\\>"
   ..............................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/progmodes/verilog-mode.el:6248:33: In call to looking-at: Duplicated alternative branch (pos 377)
  "[ \t]*\\<\\(`__FILE__\\|`__LINE\\|`begin_keywords\\|`celldefine\\|`default_nettype\\|`define\\|`else\\|`elsif\\|`end_keywords\\|`endcelldefine\\|`endif\\|`ifdef\\|`ifndef\\|`include\\|`line\\|`nounconnected_drive\\|`pragma\\|`resetall\\|`timescale\\|`unconnected_drive\\|`undef\\|`undefineall\\|`case\\|`default\\|`endfor\\|`endprotect\\|`endswitch\\|`endwhile\\|`for\\|`format\\|`if\\|`let\\|`protect\\|`switch\\|`timescale\\|`time_scale\\|`while\\)\\>"
   ..............................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/progmodes/verilog-mode.el:6313:17: In call to looking-at: Duplicated alternative branch (pos 377)
  "[ \t]*\\<\\(`__FILE__\\|`__LINE\\|`begin_keywords\\|`celldefine\\|`default_nettype\\|`define\\|`else\\|`elsif\\|`end_keywords\\|`endcelldefine\\|`endif\\|`ifdef\\|`ifndef\\|`include\\|`line\\|`nounconnected_drive\\|`pragma\\|`resetall\\|`timescale\\|`unconnected_drive\\|`undef\\|`undefineall\\|`case\\|`default\\|`endfor\\|`endprotect\\|`endswitch\\|`endwhile\\|`for\\|`format\\|`if\\|`let\\|`protect\\|`switch\\|`timescale\\|`time_scale\\|`while\\)\\>"
   ..............................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/textmodes/css-mode.el:952:32: In css-font-lock-keywords: Repetition of repetition (pos 27)
  "^[ \t]*\\(:root\\)\\(?:[\n \t]*\\)*{"
   ..................................^
lisp/textmodes/css-mode.el:952:32: In css-font-lock-keywords: Escaped non-special character `:' (pos 518)
  "^[ \t]*\\([^@/:{}() \t\n][^:{}()]*\\(?:\\(:\\(a\\(?:ctive\\|fter\\)\\|before\\|checked\\|d\\(?:efault\\|isabled\\)\\|e\\(?:mpty\\|nabled\\)\\|f\\(?:irst\\(?:-\\(?:child\\|l\\(?:etter\\|ine\\)\\|of-type\\)\\)?\\|ocus\\(?:-within\\)?\\)\\|hover\\|in\\(?:-range\\|determinate\\|valid\\)\\|l\\(?:a\\(?:ng\\|st-\\(?:child\\|of-type\\)\\)\\|eft\\|ink\\)\\|n\\(?:ot\\|th-\\(?:child\\|last-\\(?:child\\|of-type\\)\\|of-type\\)\\)\\|o\\(?:nly-\\(?:child\\|of-type\\)\\|ptional\\|ut-of-range\\)\\|r\\(?:e\\(?:ad-\\(?:only\\|write\\)\\|quired\\)\\|\\(?:igh\\|oo\\)t\\)\\|scope\\|target\\|v\\(?:\\(?:ali\\|isite\\)d\\)\\)\\|\\::\\(after\\|before\\|first-l\\(?:etter\\|ine\\)\\)\\)\\(?:([^)]+)\\)?[^:{}()\n]*\\)*\\)\\(?:\n[ \t]*\\)*{"
   ....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................^
lisp/textmodes/picture.el:428:41: Value from `regexp-quote' cannot be spliced into `[...]'
lisp/textmodes/picture.el:458:19: `regexp-quote' cannot be used for arguments to `skip-chars-backward'
lisp/textmodes/picture.el:463:30: Value from `regexp-quote' cannot be spliced into `[...]'

;; These were found using an experimental and unreleased version of xr.
lisp/vc/diff-mode.el:2215:19: In call to re-search-forward: Unintuitive range `+-<' (pos 3)
  "\n[!+-<>]\\(-- [0-9]+\\(,[0-9]+\\)? ----\n\\( .*\n\\)*[+]\\)?"
   ....^
lisp/speedbar.el:2852:42: In call to re-search-forward: Unintuitive range `+-?' (pos 21)
  "^\\([0-9]+\\):\\s-*[[<][+-?][]>] "
   ........................^
lisp/speedbar.el:2903:42: In call to re-search-forward: Unintuitive range `+-?' (pos 19)
  "^\\([0-9]+\\):\\s-*\\[[+-?]\\] "
   .......................^
lisp/woman.el:3514:26: In call to looking-at: Unintuitive range `+-/' (pos 1)
  "[+-/*%]"
   .^
lisp/net/webjump.el:345:39: In call to string-match: Two-character range `.-/' (pos 8)
  "[a-zA-Z_.-/]"
   ........^
lisp/align.el:386:3: In align-rules-list (perl-assignment): Two-character range `*-+' (pos 6)
  "[^=!^&*-+<>/| \t\n]\\(\\s-*\\)=[~>]?\\(\\s-*\\)\\([^>= \t\n]\\|$\\)"
   ......^

Re: Regexp error scan (March 26)

[Paul Eggert]

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

On 3/26/19 10:38 AM, Mattias Engdegård wrote:
> This is the latest regexp error scan of Emacs source files, using relint 1.5 (in ELPA now, or when it updates), xr 1.9.
> New checks has permitted it to discover more irregularities.
>
> The log contains, at the end, checks found with an experimental version of xr that yielded too many false positives to be useful in general, but these errors seem to be genuine.

Thanks, I installed the attached patch to try to fix those issues.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-2019-03-26-regex-cleanup.patch --]
[-- Type: text/x-patch; name="0001-2019-03-26-regex-cleanup.patch", Size: 12613 bytes --]

From d6b45e7ec0d4ced419f413ff20ce854964ae3cce Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 26 Mar 2019 19:06:36 -0700
Subject: [PATCH] 2019-03-26 regex cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problems reported by Mattias Engdegård in:
https://lists.gnu.org/r/emacs-devel/2019-03/msg01028.html
* lisp/align.el (align-rules-list):
* lisp/speedbar.el (speedbar-check-read-only, speedbar-check-vc):
* lisp/vc/diff-mode.el (diff-add-change-log-entries-other-window):
* lisp/woman.el (woman-parse-numeric-arg):
Put "-" at end of character alternatives, since a range was not intended.
* lisp/erc/erc.el (font-lock):
* lisp/mail/footnote.el (cl-seq):
Avoid duplicate character alternatives by using cl-seq API.
* lisp/mail/footnote.el (footnote--current-regexp):
* lisp/textmodes/css-mode.el (css--font-lock-keywords):
Avoid repetition of repetition.
* lisp/net/webjump.el (webjump-url-encode):
Add ~ to character alternatives, and rewrite confusing range.
* lisp/progmodes/verilog-mode.el (verilog-compiler-directives)
(verilog-assignment-operator-re):
Remove duplicate.
* lisp/progmodes/verilog-mode.el (verilog-preprocessor-re):
* lisp/textmodes/css-mode.el (css--font-lock-keywords):
Don’t escape a char that doesn’t need it.
* lisp/textmodes/picture.el (picture-tab-chars): In docstring,
do not say regexp characters will be quoted; merely say in
another way that the syntax is that of character alternatives.
(picture-set-tab-stops, picture-tab-search): Don’t attempt
to regexp-quote picture-tab-chars.
(picture-tab-search): Quote \ in picture-tab-chars for
skip-chars-backwards, which treats \ differently than
regexp character alternatives do.
---
 lisp/align.el                  |  2 +-
 lisp/erc/erc.el                |  7 +++----
 lisp/mail/footnote.el          | 16 ++++++++++++----
 lisp/net/webjump.el            |  2 +-
 lisp/progmodes/verilog-mode.el |  8 +++-----
 lisp/speedbar.el               |  4 ++--
 lisp/textmodes/css-mode.el     |  4 ++--
 lisp/textmodes/picture.el      | 14 ++++++++------
 lisp/vc/diff-mode.el           |  2 +-
 lisp/woman.el                  |  2 +-
 10 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/lisp/align.el b/lisp/align.el
index a81498be5d..fd88d0eda4 100644
--- a/lisp/align.el
+++ b/lisp/align.el
@@ -438,7 +438,7 @@ align-rules-list
      (tab-stop . nil))
 
     (perl-assignment
-     (regexp   . ,(concat "[^=!^&*-+<>/| \t\n]\\(\\s-*\\)=[~>]?"
+     (regexp   . ,(concat "[^=!^&*+<>/| \t\n-]\\(\\s-*\\)=[~>]?"
 			  "\\(\\s-*\\)\\([^>= \t\n]\\|$\\)"))
      (group    . (1 2))
      (modes    . align-perl-modes)
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index bcaa3e4525..e34487de27 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -67,6 +67,7 @@
 (load "erc-loaddefs" nil t)
 
 (eval-when-compile (require 'cl-lib))
+(require 'cl-seq)
 (require 'font-lock)
 (require 'pp)
 (require 'thingatpt)
@@ -2522,10 +2523,8 @@ erc-lurker-maybe-trim
 non-nil."
   (if erc-lurker-trim-nicks
       (replace-regexp-in-string
-       (format "[%s]"
-               (mapconcat (lambda (char)
-                            (regexp-quote (char-to-string char)))
-                          erc-lurker-ignore-chars ""))
+       (regexp-opt (cl-delete-duplicates
+		    (mapcar #'char-to-string erc-lurker-ignore-chars)))
        "" nick)
     nick))
 
diff --git a/lisp/mail/footnote.el b/lisp/mail/footnote.el
index a7802929dc..7f88e30120 100644
--- a/lisp/mail/footnote.el
+++ b/lisp/mail/footnote.el
@@ -64,6 +64,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
+(require 'cl-seq)
 (defvar filladapt-token-table)
 
 (defgroup footnote nil
@@ -363,7 +364,9 @@ footnote-hebrew-numeric
     ("ק" "ר" "ש" "ת" "תק" "תר" "תש" "תת" "תתק")))
 
 (defconst footnote-hebrew-numeric-regex
-  (concat "[" (apply #'concat (apply #'append footnote-hebrew-numeric)) "']+"))
+  (concat "[" (cl-delete-duplicates
+	       (apply #'concat (apply #'append footnote-hebrew-numeric)))
+	  "']+"))
 ;; (defconst footnote-hebrew-numeric-regex "\\([אבגדהוזחט]'\\)?\\(ת\\)?\\(ת\\)?\\([קרשת]\\)?\\([טיכלמנסעפצ]\\)?\\([אבגדהוזחט]\\)?")
 
 (defun footnote--hebrew-numeric (n)
@@ -457,9 +460,14 @@ footnote--index-to-string
 
 (defun footnote--current-regexp ()
   "Return the regexp of the index of the current style."
-  (concat (nth 2 (or (assq footnote-style footnote-style-alist)
-		     (nth 0 footnote-style-alist)))
-	  "*"))
+  (let ((regexp (nth 2 (or (assq footnote-style footnote-style-alist)
+			   (nth 0 footnote-style-alist)))))
+    (concat
+     ;; Hack to avoid repetition of repetition.
+     (if (string-match "[^\\]\\\\\\{2\\}*[*+?]\\'" regexp)
+	 (substring regexp 0 -1)
+       regexp)
+     "*")))
 
 (defun footnote--refresh-footnotes (&optional index-regexp)
   "Redraw all footnotes.
diff --git a/lisp/net/webjump.el b/lisp/net/webjump.el
index 40df23e174..e297b9d610 100644
--- a/lisp/net/webjump.el
+++ b/lisp/net/webjump.el
@@ -342,7 +342,7 @@ webjump-url-encode
   (mapconcat (lambda (c)
                (let ((s (char-to-string c)))
                  (cond ((string= s " ") "+")
-                       ((string-match "[a-zA-Z_.-/]" s) s)
+		       ((string-match "[a-zA-Z_./~-]" s) s)
                        (t (upcase (format "%%%02x" c))))))
              (encode-coding-string str 'utf-8)
              ""))
diff --git a/lisp/progmodes/verilog-mode.el b/lisp/progmodes/verilog-mode.el
index 9e241c70e7..f55cf0002d 100644
--- a/lisp/progmodes/verilog-mode.el
+++ b/lisp/progmodes/verilog-mode.el
@@ -2053,7 +2053,7 @@ verilog-compiler-directives
       "`resetall" "`timescale" "`unconnected_drive" "`undef" "`undefineall"
       ;; compiler directives not covered by IEEE 1800
       "`case" "`default" "`endfor" "`endprotect" "`endswitch" "`endwhile" "`for"
-      "`format" "`if" "`let" "`protect" "`switch" "`timescale" "`time_scale"
+      "`format" "`if" "`let" "`protect" "`switch" "`time_scale"
       "`while"
       ))
   "List of Verilog compiler directives.")
@@ -2414,9 +2414,7 @@ verilog-assignment-operator-re
      '(
        ;; blocking assignment_operator
        "=" "+=" "-=" "*=" "/=" "%=" "&=" "|=" "^=" "<<=" ">>=" "<<<=" ">>>="
-       ;; non blocking assignment operator
-       "<="
-       ;; comparison
+       ;; comparison (also nonblocking assignment "<=")
        "==" "!=" "===" "!==" "<=" ">=" "==?" "!=?" "<->"
        ;; event_trigger
        "->" "->>"
@@ -2973,7 +2971,7 @@ verilog-preprocessor-re
      "\\<\\(`pragma\\)\\>\\s-+.+$"
      "\\)\\|\\(?:"
      ;; `timescale time_unit / time_precision
-     "\\<\\(`timescale\\)\\>\\s-+10\\{0,2\\}\\s-*[munpf]?s\\s-*\\/\\s-*10\\{0,2\\}\\s-*[munpf]?s"
+     "\\<\\(`timescale\\)\\>\\s-+10\\{0,2\\}\\s-*[munpf]?s\\s-*/\\s-*10\\{0,2\\}\\s-*[munpf]?s"
      "\\)\\|\\(?:"
      ;; `define and `if can span multiple lines if line ends in '\'. NOTE: `if is not IEEE 1800-2012
      ;; from http://www.emacswiki.org/emacs/MultilineRegexp
diff --git a/lisp/speedbar.el b/lisp/speedbar.el
index 399ef4557b..4823e4ba56 100644
--- a/lisp/speedbar.el
+++ b/lisp/speedbar.el
@@ -2849,7 +2849,7 @@ speedbar-check-read-only
 	(progn
 	  (goto-char speedbar-ro-to-do-point)
 	  (while (and (not (input-pending-p))
-		      (re-search-forward "^\\([0-9]+\\):\\s-*[[<][+-?][]>] "
+		      (re-search-forward "^\\([0-9]+\\):\\s-*[[<][+?-][]>] "
 					 nil t))
 	    (setq speedbar-ro-to-do-point (point))
 	    (let ((f (speedbar-line-file)))
@@ -2900,7 +2900,7 @@ speedbar-check-vc
 	(progn
 	  (goto-char speedbar-vc-to-do-point)
 	  (while (and (not (input-pending-p))
-		      (re-search-forward "^\\([0-9]+\\):\\s-*\\[[+-?]\\] "
+		      (re-search-forward "^\\([0-9]+\\):\\s-*\\[[+?-]\\] "
 					 nil t))
 	    (setq speedbar-vc-to-do-point (point))
 	    (if (speedbar-check-vc-this-line (match-string 1))
diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index cddcdc0947..57ecc9788e 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -892,7 +892,7 @@ css--font-lock-keywords
     (,(concat "@" css-ident-re) (0 font-lock-builtin-face))
     ;; Selectors.
     ;; Allow plain ":root" as a selector.
-    ("^[ \t]*\\(:root\\)\\(?:[\n \t]*\\)*{" (1 'css-selector keep))
+    ("^[ \t]*\\(:root\\)\\(?:[\n \t]*\\){" (1 'css-selector keep))
     ;; FIXME: attribute selectors don't work well because they may contain
     ;; strings which have already been highlighted as f-l-string-face and
     ;; thus prevent this highlighting from being applied (actually now that
@@ -915,7 +915,7 @@ css--font-lock-keywords
        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                        css-pseudo-element-ids)
                                t)
-       "\\|\\::" (regexp-opt css-pseudo-element-ids t) "\\)"
+       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"
        "\\(?:([^)]+)\\)?"
        (if (not sassy)
            "[^:{}()\n]*"
diff --git a/lisp/textmodes/picture.el b/lisp/textmodes/picture.el
index f0e30135f1..b520849467 100644
--- a/lisp/textmodes/picture.el
+++ b/lisp/textmodes/picture.el
@@ -387,7 +387,8 @@ picture-tab-chars
 \\[picture-set-tab-stops] and \\[picture-tab-search].
 The syntax for this variable is like the syntax used inside of `[...]'
 in a regular expression--but without the `[' and the `]'.
-It is NOT a regular expression, any regexp special characters will be quoted.
+It is NOT a regular expression, and should follow the usual
+rules for the contents of a character alternative.
 It defines a set of \"interesting characters\" to look for when setting
 \(or searching for) tab stops, initially \"!-~\" (all printing characters).
 For example, suppose that you are editing a table which is formatted thus:
@@ -425,7 +426,7 @@ picture-set-tab-stops
       (if arg
 	  (setq tabs (or (default-value 'tab-stop-list)
 			 (indent-accumulate-tab-stops (window-width))))
-	(let ((regexp (concat "[ \t]+[" (regexp-quote picture-tab-chars) "]")))
+	(let ((regexp (concat "[ \t]+[" picture-tab-chars "]")))
 	  (beginning-of-line)
 	  (let ((bol (point)))
 	    (end-of-line)
@@ -433,8 +434,8 @@ picture-set-tab-stops
 	      (skip-chars-forward " \t")
 	      (setq tabs (cons (current-column) tabs)))
 	    (if (null tabs)
-		(error "No characters in set %s on this line"
-		       (regexp-quote picture-tab-chars))))))
+		(error "No characters in set [%s] on this line"
+		       picture-tab-chars)))))
       (setq tab-stop-list tabs)
       (let ((blurb (make-string (1+ (nth (1- (length tabs)) tabs)) ?\ )))
 	(while tabs
@@ -455,12 +456,13 @@ picture-tab-search
 	       (progn
 		 (beginning-of-line)
 		 (skip-chars-backward
-		  (concat "^" (regexp-quote picture-tab-chars))
+		  (concat "^" (replace-regexp-in-string
+			       "\\\\" "\\\\" picture-tab-chars nil t))
 		  (point-min))
 		 (not (bobp))))
 	  (move-to-column target))
       (if (re-search-forward
-	   (concat "[ \t]+[" (regexp-quote picture-tab-chars) "]")
+	   (concat "[ \t]+[" picture-tab-chars "]")
 	   (line-end-position)
 	   'move)
 	  (setq target (1- (current-column)))
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index b67caab7f5..dbde284da8 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2213,7 +2213,7 @@ diff-add-change-log-entries-other-window
                  ;; `add-change-log-entry-other-window' works better in
                  ;; that case.
                  (re-search-forward
-                  (concat "\n[!+-<>]"
+		  (concat "\n[!+<>-]"
                           ;; If the hunk is a context hunk with an empty first
                           ;; half, recognize the "--- NNN,MMM ----" line
                           "\\(-- [0-9]+\\(,[0-9]+\\)? ----\n"
diff --git a/lisp/woman.el b/lisp/woman.el
index a351f788ec..39d9b806d2 100644
--- a/lisp/woman.el
+++ b/lisp/woman.el
@@ -3511,7 +3511,7 @@ woman-parse-numeric-arg
   (let ((value (if (looking-at "[+-]") 0 (woman-parse-numeric-value)))
 	op)
     (while (cond
-	    ((looking-at "[+-/*%]")	; arithmetic operators
+	    ((looking-at "[+/*%-]")	; arithmetic operators
 	     (forward-char)
 	     (setq op (intern-soft (match-string 0)))
 	     (setq value (funcall op value (woman-parse-numeric-value))))
-- 
2.20.1

Re: Regexp error scan (March 26)

[Basil L. Contovounesios]

Paul Eggert <eggert@cs.ucla.edu> writes:

> diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
> index bcaa3e4525..e34487de27 100644
> --- a/lisp/erc/erc.el
> +++ b/lisp/erc/erc.el
> @@ -67,6 +67,7 @@
>  (load "erc-loaddefs" nil t)
>  
>  (eval-when-compile (require 'cl-lib))
> +(require 'cl-seq)
>  (require 'font-lock)
>  (require 'pp)
>  (require 'thingatpt)
> @@ -2522,10 +2523,8 @@ erc-lurker-maybe-trim
>  non-nil."
>    (if erc-lurker-trim-nicks
>        (replace-regexp-in-string
> -       (format "[%s]"
> -               (mapconcat (lambda (char)
> -                            (regexp-quote (char-to-string char)))
> -                          erc-lurker-ignore-chars ""))
> +       (regexp-opt (cl-delete-duplicates
> +		    (mapcar #'char-to-string erc-lurker-ignore-chars)))
>         "" nick)
>      nick))

regexp-opt already deletes duplicates, so you can just:

(regexp-opt (mapcar #'char-to-string erc-lurker-ignore-chars))

Besides, cl-delete-duplicates defaults to eql for equality, so it's a
no-op here anyway.

Thanks,

-- 
Basil

Re: Regexp error scan (March 26)

[Mattias Engdegård]

27 mars 2019 kl. 04.43 skrev Basil L. Contovounesios <contovob@tcd.ie>:
> 
> Paul Eggert <eggert@cs.ucla.edu> writes:
> 
>>       (replace-regexp-in-string
>> -       (format "[%s]"
>> -               (mapconcat (lambda (char)
>> -                            (regexp-quote (char-to-string char)))
>> -                          erc-lurker-ignore-chars ""))
>> +       (regexp-opt (cl-delete-duplicates
>> +		    (mapcar #'char-to-string erc-lurker-ignore-chars)))
>>        "" nick)
>>     nick))
> 
> regexp-opt already deletes duplicates, so you can just:
> 
> (regexp-opt (mapcar #'char-to-string erc-lurker-ignore-chars))
> 
> Besides, cl-delete-duplicates defaults to eql for equality, so it's a
> no-op here anyway.

Or perhaps:

(regexp-opt-charset (append erc-lurker-ignore-chars nil))

Re: Regexp error scan (March 26)

[Mattias Engdegård]

27 mars 2019 kl. 03.10 skrev Paul Eggert <eggert@cs.ucla.edu>:
> 
> Thanks, I installed the attached patch to try to fix those issues.

Thank you, that was quick!

-  (concat (nth 2 (or (assq footnote-style footnote-style-alist)
-		     (nth 0 footnote-style-alist)))
-	  "*"))
+  (let ((regexp (nth 2 (or (assq footnote-style footnote-style-alist)
+			   (nth 0 footnote-style-alist)))))
+    (concat
+     ;; Hack to avoid repetition of repetition.
+     (if (string-match "[^\\]\\\\\\{2\\}*[*+?]\\'" regexp)
+	 (substring regexp 0 -1)
+       regexp)
+     "*")))
 
The repetition-of-repetition check is useful and has caught several
actual errors; this may be the first true false positive that has to
be hacked around. We could suppress the complaint if the inner
repetition is wrapped in brackets, like \(?:a+\)*, but it would reduce
the error-finding power of the check.

In this case I'm not sure the added * makes sense at all; there is
always a single number within the footnote-{start,end}-tag pairs.
Worse, the code goes on and adds yet another + later on, in
footnote-refresh-footnotes, just in case. That makes even less sense.
Likely, both the * and the extra + should go away.

-    ("^[ \t]*\\(:root\\)\\(?:[\n \t]*\\)*{" (1 'css-selector keep))
+    ("^[ \t]*\\(:root\\)\\(?:[\n \t]*\\){" (1 'css-selector keep))

Those brackets around the [\n \t] could go away, too.

Re: Regexp error scan (March 26)

[Andy Moreton]

On Wed 27 Mar 2019, Mattias Engdegård wrote:

> 27 mars 2019 kl. 04.43 skrev Basil L. Contovounesios <contovob@tcd.ie>:
>> 
>> Paul Eggert <eggert@cs.ucla.edu> writes:
>> 
>>>       (replace-regexp-in-string
>>> -       (format "[%s]"
>>> -               (mapconcat (lambda (char)
>>> -                            (regexp-quote (char-to-string char)))
>>> -                          erc-lurker-ignore-chars ""))
>>> +       (regexp-opt (cl-delete-duplicates
>>> +		    (mapcar #'char-to-string erc-lurker-ignore-chars)))
>>>        "" nick)
>>>     nick))
>> 
>> regexp-opt already deletes duplicates, so you can just:
>> 
>> (regexp-opt (mapcar #'char-to-string erc-lurker-ignore-chars))
>> 
>> Besides, cl-delete-duplicates defaults to eql for equality, so it's a
>> no-op here anyway.
>
> Or perhaps:
>
> (regexp-opt-charset (append erc-lurker-ignore-chars nil))

This *-charset naming is helpful, as it reminds users of the expected
argument type.

Given the previously noted problems with incorrect arguments for
`skip-chars-forward' and `skip-chars-backward', would it make sense to
rename these functions to `forward-charset' and `backward-charset' ?

    AndyM
  

Re: Regexp error scan (March 26)

[Stefan Monnier]

>> (regexp-opt-charset (append erc-lurker-ignore-chars nil))

Why do we need the append-nil thingy?
Does regexp-opt-charset modify its argument by side-effect?
If so, we should change that,


        Stefan

Re: Regexp error scan (March 26)

[Mattias Engdegård]

27 mars 2019 kl. 15.36 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
> 
>>> (regexp-opt-charset (append erc-lurker-ignore-chars nil))
> 
> Why do we need the append-nil thingy?
> Does regexp-opt-charset modify its argument by side-effect?

No, it's a way to convert a string to a list of characters which is what regexp-opt-charset expects.
(mapcar #'identity ...) would also work, but this is a little shorter.

Re: Regexp error scan (March 26)

[Stefan Monnier]

>>>> (regexp-opt-charset (append erc-lurker-ignore-chars nil))
>> Why do we need the append-nil thingy?
>> Does regexp-opt-charset modify its argument by side-effect?
>
> No, it's a way to convert a string to a list of characters which is what regexp-opt-charset expects.
> (mapcar #'identity ...) would also work, but this is a little shorter.

Ah, good.  But it would deserve a comment,


        Stefan

Re: Regexp error scan (March 26)

[Noam Postavsky]

On Wed, 27 Mar 2019 at 10:46, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> >>>> (regexp-opt-charset (append erc-lurker-ignore-chars nil))
> >> Why do we need the append-nil thingy?
> >> Does regexp-opt-charset modify its argument by side-effect?
> >
> > No, it's a way to convert a string to a list of characters which is what regexp-opt-charset expects.
> > (mapcar #'identity ...) would also work, but this is a little shorter.
>
> Ah, good.  But it would deserve a comment,

Or use string-to-list?

Re: Regexp error scan (March 26)

[Damien Collard]

On Mer, mar 27 2019, Mattias Engdegård wrote:

> 27 mars 2019 kl. 15.36 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>> 
>>>> (regexp-opt-charset (append erc-lurker-ignore-chars nil))
>> 
>> Why do we need the append-nil thingy?
>> Does regexp-opt-charset modify its argument by side-effect?
>
> No, it's a way to convert a string to a list of characters which is what regexp-opt-charset expects.
> (mapcar #'identity ...) would also work, but this is a little shorter.

Hello,

Why not use `string-to-list` then? It is implemented as (append ... nil)
and would be much clearer imho.

Damien

Re: Regexp error scan (March 26)

[Mattias Engdegård]

27 mars 2019 kl. 15.49 skrev Noam Postavsky <npostavs@gmail.com>:
> 
> On Wed, 27 Mar 2019 at 10:46, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> 
>> Ah, good.  But it would deserve a comment,
> 
> Or use string-to-list?

Thanks, I forgot about string-to-list -- it's a comparatively recent addition.
It's longer, but not if you count the comment that Stefan insisted on!

Re: Regexp error scan (March 26)

[Stefan Monnier]

> Or use string-to-list?

That would be too obvious.


        Stefan

Re: Regexp error scan (March 26)

[Paul Eggert]

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

On 3/27/19 8:48 AM, Stefan Monnier wrote:
>> Or use string-to-list?
> That would be too obvious.

Thanks to everybody who helped improve that code with "obvious" changes
that weren't obvious to me. I installed the attached patches to try to
incorporate all the comments. I'm not sure what to do about
footnote.el's blithe overuse of "+" and "*" so I merely left a FIXME
comment for that, stealing its wording from Mattias's email.

I avoided regexp-opt before because its doc string implied that
(regexp-opt '("a" "a")) was invalid. The first patch attempts to fix
that confusion too.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-regexp-opt-charset-to-improve-regexp-tweaks.patch --]
[-- Type: text/x-patch; name="0001-Use-regexp-opt-charset-to-improve-regexp-tweaks.patch", Size: 3653 bytes --]

From 92acab73e0dd3921b53eac4f3fba327b7aa4d3aa Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 27 Mar 2019 11:36:13 -0700
Subject: [PATCH] Use regexp-opt-charset to improve regexp tweaks

* lisp/emacs-lisp/regexp-opt.el (regexp-opt):
Reword confusing sentence in doc string.
* lisp/erc/erc.el (erc-lurker-maybe-trim):
* lisp/mail/footnote.el (footnote-hebrew-numeric-regex):
Improve by using regexp-opt-charset.
---
 lisp/emacs-lisp/regexp-opt.el |  6 +++---
 lisp/erc/erc.el               |  4 +---
 lisp/mail/footnote.el         | 12 ++++++++----
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/lisp/emacs-lisp/regexp-opt.el b/lisp/emacs-lisp/regexp-opt.el
index fce6a47d98..d883752d71 100644
--- a/lisp/emacs-lisp/regexp-opt.el
+++ b/lisp/emacs-lisp/regexp-opt.el
@@ -86,9 +86,9 @@
 ;;;###autoload
 (defun regexp-opt (strings &optional paren keep-order)
   "Return a regexp to match a string in the list STRINGS.
-Each string should be unique in STRINGS and should not contain
-any regexps, quoted or not.  Optional PAREN specifies how the
-returned regexp is surrounded by grouping constructs.
+Each member of STRINGS is treated as a fixed string, not as a regexp.
+Optional PAREN specifies how the returned regexp is surrounded by
+grouping constructs.
 
 If STRINGS is the empty list, the return value is a regexp that
 never matches anything.
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index e34487de27..d1fa5c7f12 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -67,7 +67,6 @@
 (load "erc-loaddefs" nil t)
 
 (eval-when-compile (require 'cl-lib))
-(require 'cl-seq)
 (require 'font-lock)
 (require 'pp)
 (require 'thingatpt)
@@ -2523,8 +2522,7 @@ erc-lurker-maybe-trim
 non-nil."
   (if erc-lurker-trim-nicks
       (replace-regexp-in-string
-       (regexp-opt (cl-delete-duplicates
-		    (mapcar #'char-to-string erc-lurker-ignore-chars)))
+       (regexp-opt-charset (string-to-list erc-lurker-ignore-chars))
        "" nick)
     nick))
 
diff --git a/lisp/mail/footnote.el b/lisp/mail/footnote.el
index 7f88e30120..81dc11de76 100644
--- a/lisp/mail/footnote.el
+++ b/lisp/mail/footnote.el
@@ -64,7 +64,6 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
-(require 'cl-seq)
 (defvar filladapt-token-table)
 
 (defgroup footnote nil
@@ -364,9 +363,9 @@ footnote-hebrew-numeric
     ("ק" "ר" "ש" "ת" "תק" "תר" "תש" "תת" "תתק")))
 
 (defconst footnote-hebrew-numeric-regex
-  (concat "[" (cl-delete-duplicates
-	       (apply #'concat (apply #'append footnote-hebrew-numeric)))
-	  "']+"))
+  (let ((numchars (string-to-list
+		   (apply #'concat (apply #'append footnote-hebrew-numeric)))))
+    (concat (regexp-opt-charset (cons ?' numchars)) "+")))
 ;; (defconst footnote-hebrew-numeric-regex "\\([אבגדהוזחט]'\\)?\\(ת\\)?\\(ת\\)?\\([קרשת]\\)?\\([טיכלמנסעפצ]\\)?\\([אבגדהוזחט]\\)?")
 
 (defun footnote--hebrew-numeric (n)
@@ -464,6 +463,11 @@ footnote--current-regexp
 			   (nth 0 footnote-style-alist)))))
     (concat
      ;; Hack to avoid repetition of repetition.
+     ;; FIXME: I'm not sure the added * makes sense at all; there is
+     ;; always a single number within the footnote-{start,end}-tag pairs.
+     ;; Worse, the code goes on and adds yet another + later on, in
+     ;; footnote-refresh-footnotes, just in case. That makes even less sense.
+     ;; Likely, both the * and the extra + should go away.
      (if (string-match "[^\\]\\\\\\{2\\}*[*+?]\\'" regexp)
 	 (substring regexp 0 -1)
        regexp)
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Tune-css-mode-regexp.patch --]
[-- Type: text/x-patch; name="0001-Tune-css-mode-regexp.patch", Size: 1288 bytes --]

From df167575d1ac2d056c8a2ef1fc83d768c09a3d28 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 27 Mar 2019 11:43:18 -0700
Subject: [PATCH] Tune css-mode regexp
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/textmodes/css-mode.el (css--font-lock-keywords):
Omit unnecessary \(?: \) in regexp.  Suggested by Mattias Engdegård in:
https://lists.gnu.org/r/emacs-devel/2019-03/msg01042.html
---
 lisp/textmodes/css-mode.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index d3ca2d9558..11a77b5bb7 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -892,7 +892,7 @@ css--font-lock-keywords
     (,(concat "@" css-ident-re) (0 font-lock-builtin-face))
     ;; Selectors.
     ;; Allow plain ":root" as a selector.
-    ("^[ \t]*\\(:root\\)\\(?:[\n \t]*\\){" (1 'css-selector keep))
+    ("^[ \t]*\\(:root\\)[\n \t]*{" (1 'css-selector keep))
     ;; FIXME: attribute selectors don't work well because they may contain
     ;; strings which have already been highlighted as f-l-string-face and
     ;; thus prevent this highlighting from being applied (actually now that
-- 
2.20.1