GNU bug report logs - #34805
25.2; smie-indent-fixindent wrong if comment-start-skip has alternatives

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 34805 in the body.
You can then email your comments to 34805 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andrey Bondarenko <abone27 <at> mail.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.2; smie-indent-fixindent wrong if comment-start-skip has alternatives
Date: Sun, 10 Mar 2019 16:23:05 +0300

If comment-start-skip contains alternatives smie-indent-fixindent will
construct wrong regexp that appends "fixindent" only to last alternative
and match all other aletrnatives as is.

For example: 

(let ((comment-start-skip "#+\\s *\\|//+\\s *")
      (comment-end-skip ""))
 (concat comment-start-skip
         "fixindent" 
         comment-end-skip)) → "#+\\s *\\|//+\\s *fixindent"

This regexp would match "// fixindent" comments and any comment
that starts with "#", which is wrong. 

I think it should enclose comment-start-skip and comment-end-skip in \\(
\\) to work correctly. Something like:

(let ((comment-start-skip "#+\\s *\\|//+\\s *")
      (comment-end-skip ""))
 (concat "\\(" comment-start-skip "\\)"
         "fixindent" 
         "\\(" comment-end-skip "\\)")) → "\\(#+\\s *\\|//+\\s *\\)fixindent\\(\\)"




In GNU Emacs 25.2.2 (x86_64-pc-linux-gnu, GTK+ Version 3.22.21)
 of 2017-09-23, modified by Debian built on lgw01-amd64-050
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description:	Ubuntu 18.04.2 LTS

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --build x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.2/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-x=yes --with-x-toolkit=gtk3
 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
 -fdebug-prefix-map=/build/emacs25-jYekUr/emacs25-25.2+1=. -fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: ru_RU.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Prog

Minor modes in effect:
  diff-auto-refine-mode: t
  recentf-mode: t
  delete-selection-mode: t
  ergoemacs-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-ethan-wspace-mode: t
  show-paren-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
24 matches
Mark set [2 times]
2 matches
Mark set
2 matches [9 times]
24 matches
200 matches (limit reached)
Mark set
18 matches
ergoemacs-command-loop--grow-interactive: End of buffer [3 times]

Load-path shadows:
/usr/share/emacs/25.2/site-lisp/debian-startup hides /usr/share/emacs/site-lisp/debian-startup

Features:
(shadow sort mail-extr emacsbug message dired rfc822 mml mml-sec epg
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mail-utils vc-hg
taskjuggler-mode derived skeleton warnings vc-git diff-mode edebug
re-builder smie misearch multi-isearch recentf tree-widget delsel
ergoemacs-themes edmacro two-column cus-edit wid-edit
ergoemacs-translate ergoemacs-theme-engine ergoemacs-mapkeymap
ergoemacs-map-properties ergoemacs-map ergoemacs-lib ergoemacs-layouts
ergoemacs-key-description descr-text ergoemacs-functions
ergoemacs-component ergoemacs-advice ergoemacs-command-loop
ergoemacs-macros jka-compr persistent-soft list-utils pcache
eieio-compat eieio-base printing ps-print ps-def lpr kmacro
ergoemacs-mode undo-tree diff ethan-wspace paren cus-start cus-load
finder-inf info package epg-config ido seq ess-toolbar ess-mouse mouseme
thingatpt browse-url ess-swv ess-noweb ess-noweb-font-lock-mode
ess-bugs-l ess-sas-d ess-sas-l ess-sas-a ess-stata-mode ess-stata-lang
cc-vars cc-defs make-regexp essd-els ess-sp6-d ess-dde ess-sp3-d
ess-julia julia-mode ert pp find-func ewoc debug ess-r-mode
ess-r-package ess-r-syntax ess-r-completion ess-roxy essddr ess-rd
noutline outline easy-mmode hideshow ess-s-lang ess-help ess ess-mode
ess-noweb-mode ess-inf ess-tracebug compile tramp tramp-compat
auth-source cl-seq eieio byte-opt bytecomp byte-compile cl-extra cconv
eieio-core gnus-util mm-util help-fns help-mode mail-prsvr
password-cache tramp-loaddefs cl-macs trampver ucs-normalize shell
pcomplete comint ansi-color ring format-spec advice ess-generics cl gv
cl-loaddefs pcase cl-lib ess-custom executable easymenu ess-compat
ess-utils ess-site time-date mule-util cyril-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 501591 23787)
 (symbols 48 48364 1)
 (miscs 40 261 352)
 (strings 32 124966 14441)
 (string-bytes 1 3094860)
 (vectors 16 59098)
 (vector-slots 8 1089256 41639)
 (floats 8 1576 804)
 (intervals 56 3488 0)
 (buffers 976 26))

Message #8 received at 34805 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Andrey Bondarenko <abone27 <at> mail.ru>
Cc: 34805 <at> debbugs.gnu.org
Subject: Re: bug#34805: 25.2;
 smie-indent-fixindent wrong if comment-start-skip has alternatives
Date: Thu, 11 Apr 2019 23:52:16 -0400

[Message part 1 (text/plain, inline)]

tags 34805 + patch
quit

Andrey Bondarenko <abone27 <at> mail.ru> writes:

> If comment-start-skip contains alternatives smie-indent-fixindent will
> construct wrong regexp that appends "fixindent" only to last alternative
> and match all other aletrnatives as is.
>
> For example: 
>
> (let ((comment-start-skip "#+\\s *\\|//+\\s *")
>       (comment-end-skip ""))
>  (concat comment-start-skip
>          "fixindent" 
>          comment-end-skip)) → "#+\\s *\\|//+\\s *fixindent"
>
> This regexp would match "// fixindent" comments and any comment
> that starts with "#", which is wrong. 
>
> I think it should enclose comment-start-skip and comment-end-skip in \\(
> \\) to work correctly.

I used a shy group \\(?:\\), but yes that makes sense.  I found a couple
of other spots by grepping, fixed in the patch below.

[v1-0001-Properly-bracket-concat-of-comment-start-skip-Bug.patch (text/x-diff, inline)]

From ea144b28028f6ef72d67d84f550e1798c61347e2 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> users.sourceforge.net>
Date: Fri, 5 Apr 2019 08:00:09 -0400
Subject: [PATCH v1] Properly bracket concat of comment-start-skip (Bug#34805)

* lisp/emacs-lisp/smie.el (smie-indent-fixindent):
* lisp/cedet/semantic/doc.el (semantic-doc-snarf-comment-for-tag):
* lisp/progmodes/fortran.el (fortran-previous-statement)
(fortran-next-statement)
(fortran-fill-statement):
* lisp/progmodes/vhdl-mode.el (vhdl-beginning-of-statement): Bracket
comment-start-skip and comment-end-skip to avoid unexpected regexp
operator precedence.
---
 lisp/cedet/semantic/doc.el  | 3 ++-
 lisp/emacs-lisp/smie.el     | 4 ++--
 lisp/progmodes/fortran.el   | 9 ++++++---
 lisp/progmodes/vhdl-mode.el | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/lisp/cedet/semantic/doc.el b/lisp/cedet/semantic/doc.el
index 5611629c14..4f98cf4102 100644
--- a/lisp/cedet/semantic/doc.el
+++ b/lisp/cedet/semantic/doc.el
@@ -103,7 +103,8 @@ (defun semantic-doc-snarf-comment-for-tag (nosnarf)
 	    nil
 	  ;; ok, try to clean the text up.
 	  ;; Comment start thingy
-	  (while (string-match (concat "^\\s-*" comment-start-skip) ct)
+	  (while (string-match (concat "^\\s-*\\(?:" comment-start-skip "\\)")
+                               ct)
 	    (setq ct (concat (substring ct 0 (match-beginning 0))
 			     (substring ct (match-end 0)))))
 	  ;; Arbitrary punctuation at the beginning of each line.
diff --git a/lisp/emacs-lisp/smie.el b/lisp/emacs-lisp/smie.el
index 92b639d71e..e0293c3cbb 100644
--- a/lisp/emacs-lisp/smie.el
+++ b/lisp/emacs-lisp/smie.el
@@ -1446,9 +1446,9 @@ (defun smie-indent-fixindent ()
   (and (smie-indent--bolp)
        (save-excursion
          (comment-normalize-vars)
-         (re-search-forward (concat comment-start-skip
+         (re-search-forward (concat "\\(?:" comment-start-skip "\\)"
                                     "fixindent"
-                                    comment-end-skip)
+                                    "\\(?:" comment-end-skip "\\)")
                             ;; 1+ to account for the \n comment termination.
                             (1+ (line-end-position)) t))
        (current-column)))
diff --git a/lisp/progmodes/fortran.el b/lisp/progmodes/fortran.el
index 152667040f..f01e866f55 100644
--- a/lisp/progmodes/fortran.el
+++ b/lisp/progmodes/fortran.el
@@ -1275,7 +1275,8 @@ (defun fortran-previous-statement ()
                      (concat "[ \t]*"
                              (regexp-quote fortran-continuation-string)))
                     (looking-at "[ \t]*$\\| \\{5\\}[^ 0\n]\\|\t[1-9]")
-                    (looking-at (concat "[ \t]*" comment-start-skip)))))
+                    (looking-at (concat "[ \t]*\\(?:"
+                                        comment-start-skip "\\)")))))
     (cond ((and continue-test
                 (not not-first-statement))
            (message "Incomplete continuation statement."))
@@ -1298,7 +1299,8 @@ (defun fortran-next-statement ()
                 (or (looking-at fortran-comment-line-start-skip)
                     (looking-at fortran-directive-re)
                     (looking-at "[ \t]*$\\|     [^ 0\n]\\|\t[1-9]")
-                    (looking-at (concat "[ \t]*" comment-start-skip)))))
+                    (looking-at (concat "[ \t]*\\(?:"
+                                        comment-start-skip "\\)")))))
     (if (not not-last-statement)
         'last-statement)))
 
@@ -2146,7 +2148,8 @@ (defun fortran-fill-statement ()
               (or (looking-at "[ \t]*$")
                   (looking-at fortran-comment-line-start-skip)
                   (and comment-start-skip
-                       (looking-at (concat "[ \t]*" comment-start-skip)))))
+                       (looking-at (concat "[ \t]*\\(?:"
+                                           comment-start-skip "\\)")))))
       (save-excursion
         ;; Find beginning of statement.
         (fortran-next-statement)
diff --git a/lisp/progmodes/vhdl-mode.el b/lisp/progmodes/vhdl-mode.el
index 1dc0c61d06..13d0cfa67e 100644
--- a/lisp/progmodes/vhdl-mode.el
+++ b/lisp/progmodes/vhdl-mode.el
@@ -6699,7 +6699,7 @@ (defun vhdl-beginning-of-statement (&optional count lim interactive)
     (if (and interactive
 	     (or (nth 3 state)
 		 (nth 4 state)
-		 (looking-at (concat "[ \t]*" comment-start-skip))))
+		 (looking-at (concat "[ \t]*\\(?:" comment-start-skip "\\)"))))
 	(forward-sentence (- count))
       (while (> count 0)
 	(vhdl-beginning-of-statement-1 lim)
-- 
2.11.0

Message #13 received at 34805 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Andrey Bondarenko <abone27 <at> mail.ru>
Cc: 34805 <at> debbugs.gnu.org
Subject: Re: bug#34805: 25.2;
 smie-indent-fixindent wrong if comment-start-skip has alternatives
Date: Mon, 15 Apr 2019 20:35:27 -0400

tags 34805 fixed
close 34805 27.1
quit

Noam Postavsky <npostavs <at> gmail.com> writes:

> I used a shy group \\(?:\\), but yes that makes sense.  I found a couple
> of other spots by grepping, fixed in the patch below.

Pushed to master.

7a382383ac 2019-04-15T20:28:03-04:00 "Properly bracket concat of comment-start-skip (Bug#34805)"

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.