bug#25150: 26.0.50; sexp motion commands don't work reliably in message-mode

bug#25150: 26.0.50; sexp motion commands don't work reliably in message-mode

[Sean McAfee]

Starting from "emacs -Q":

Open a new buffer.

M-x message-mode.

Enter the following two lines of text:

{"":[
]}

Press C-M-b (backward-sexp).

Point should move to the opening brace, but instead it moves to the opening square bracket.

Move point to the opening brace and press C-M-f (forward-sexp).

Point should move to the closing brace, but an "unbalanced parentheses" error is raised instead.

If the two lines are joined:

{"":[]}

...then both commands work as expected.

If you enter the two-line text above and THEN enter message-mode, the two sexp motion commands work as expected.  But if you then enter any additional single character in the buffer, the commands behave incorrectly again, as described above.



In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2016-10-06 built on sean-virtualbox
Repository revision: d48369db9c97b6f2accf702e5bbe0bda11cb92a1
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 16.04.1 LTS

Recent messages:
Mark set [2 times]
forward-sexp: Scan error: "Unbalanced parentheses", 157, 165
Mark set
next-line: End of buffer
Mark set
Mark activated
next-line: End of buffer
Mark set
forward-sexp: Scan error: "Unbalanced parentheses", 1, 7
previous-line: Beginning of buffer
forward-sexp: Scan error: "Unbalanced parentheses", 157, 165
Configured using:
 'configure --with-xwidgets'

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

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Summary

Minor modes in effect:
  recentf-mode: t
  global-anzu-mode: t
  anzu-mode: t
  volatile-highlights-mode: t
  dtrt-indent-mode: t
  projectile-global-mode: t
  projectile-mode: t
  diff-auto-refine-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  shell-dirtrack-mode: t
  google-this-mode: t
  global-flycheck-mode: t
  minibuffer-depth-indicate-mode: t
  ido-everywhere: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  override-global-mode: t
  show-paren-mode: t
  electric-pair-mode: t
  savehist-mode: t
  desktop-save-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-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
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/smcafee1/.emacs.d/elpa/svg-0.1/svg hides /usr/local/share/emacs/26.0.50/lisp/svg
/home/smcafee1/.emacs.d/elpa/scala-mode-20160902.525/ob-scala hides /usr/local/share/emacs/26.0.50/lisp/org/ob-scala
~/emacs/japanese hides /usr/local/share/emacs/26.0.50/lisp/language/japanese

Features:
(shadow emacsbug mailalias sort pcmpl-unix eieio-opt speedbar sb-image
ezimage dframe debug cus-edit cus-start cus-load proced tabify
org-datetree image-file org-capture gnus-async align gnus-cite mail-extr
nnir gnus-ml disp-table nndraft nnmh nnfolder utf-7 gnus-agent gnus-srvr
gnus-score score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view
mml-smime smime dig nntp gnus-cache gnus-sum gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo
parse-time gnus-spec gnus-int gnus-range gnus-win pulse vc-annotate
vc-bzr vc-src vc-sccs vc-cvs vc-rcs mc-edit-lines ag vc-svn find-dired
url-cache url-http url-auth vc vc-dispatcher bookmark misearch
multi-isearch network-stream nsm starttls tls gnutls executable recentf
tree-widget linum python-el-fgallina-expansions python rng-xsd
xsd-regexp rng-cmpct nxml-mode-expansions rng-nxml rng-valid rng-loc
rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap nxml-util nxml-enc xmltok perl6-mode
perl6-indent smie perl6-font-lock org-element org-rmail org-mhe org-irc
org-info org-gnus org-docview doc-view jka-compr image-mode org-bibtex
bibtex org-bbdb org-w3m the-org-mode-expansions org org-macro
org-footnote org-pcomplete org-list org-faces org-entities org-version
ob-emacs-lisp ob ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys
ob-comint ob-core ob-eval org-compat org-macs org-loaddefs cal-menu
calendar cal-loaddefs json-mode json-reformat json-snatcher
js-mode-expansions js tagedit html-mode-expansions sgml-mode dom
cc-mode-expansions cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs gnus-dired vc-git cap-words superword
subword company-oddmuse company-keywords company-etags company-gtags
company-dabbrev-code company-dabbrev company-files company-capf
company-cmake company-xcode company-clang company-semantic company-eclim
company-template company-css company-nxml company-bbdb ensime-company
yasnippet company scala-mode scala-mode-prettify-symbols
scala-mode-imenu scala-mode-map scala-mode-fontlock scala-mode-indent
scala-mode-paragraph scala-mode-lib anzu perl6-detect restclient edbi
sql view epc ctable concurrent deferred smtpmail-multi smtpmail
whitespace ensime ensime-sbt sbt-mode sbt-mode-rgrep sbt-mode-comint
sbt-mode-buffer sbt-mode-project sbt-mode-vars ensime-http ensime-ui
ensime-semantic-highlight ensime-doc ensime-search ensime-undo
ensime-startup ensime-refactor ensime-popup ensime-notes ensime-model
ensime-mode ensime-inspector imenu ensime-goto-testfile ensime-editor
popup ensime-debug gdb-mi bindat gud ensime-stacktrace ensime-inf
ensime-completion-util scala-mode-syntax ensime-config ensime-util
ensime-client ensime-vars hideshow pp arc-mode archive-mode url-gw
ensime-macros dash-functional volatile-highlights iedit iedit-lib
multiple-cursors-core rect dtrt-indent projectile grep ibuf-ext ibuffer
ibuffer-loaddefs elisp-slime-nav magit-blame magit-stash magit-bisect
magit-remote magit-commit magit-sequence magit magit-apply magit-wip
magit-log magit-diff smerge-mode diff-mode magit-core magit-autorevert
autorevert filenotify magit-process magit-popup magit-mode magit-git crm
magit-section magit-utils git-commit help-fns radix-tree log-edit
message sendmail puny rfc822 mml mml-sec epa derived epg mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
pcvs-util add-log with-editor async-bytecomp async tramp-sh tramp
tramp-compat tramp-loaddefs trampver shell pcomplete format-spec
google-this url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf mailcap flycheck find-func gnus nnheader subr-x
gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums mail-utils
mm-util mail-prsvr wid-edit haskell-interactive-mode
haskell-presentation-mode haskell-collapse haskell-process
haskell-session url-util haskell-navigate-imports haskell-compile
haskell-mode haskell-cabal haskell-utils haskell-font-lock
haskell-indentation haskell-string haskell-sort-imports haskell-lexeme
haskell-align-imports haskell-compat haskell-complete-module
haskell-ghc-support noutline outline flymake etags xref project dabbrev
haskell-customize autoinsert misc sanityinc-tomorrow-blue-theme
color-theme-sanityinc-tomorrow mb-depth ido dired-details+ dired
dired-loaddefs dired-details change-inner expand-region
text-mode-expansions er-basic-expansions expand-region-core advice
expand-region-custom org-bullets undo-tree diff ack compile paredit
edmacro kmacro use-package diminish bind-key easy-mmode finder-inf
gh-common gh-profile s ucs-normalize marshal eieio-compat ht json map
dash rx info package epg-config url-handlers url-parse auth-source eieio
eieio-core eieio-loaddefs password-cache url-vars seq byte-opt bytecomp
byte-compile cl-extra help-mode easymenu cconv paren elec-pair savehist
desktop frameset server hippie-exp comint ansi-color ring thingatpt
cl-seq perl-mode cl-macs cl gv cl-loaddefs pcase cl-lib time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win 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
term/tty-colors 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
obarray 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
xwidget-internal move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 936252 96426)
 (symbols 48 68963 3)
 (miscs 40 20187 1648)
 (strings 32 209317 35304)
 (string-bytes 1 6619591)
 (vectors 16 105692)
 (vector-slots 8 2288221 97129)
 (floats 8 1125 1820)
 (intervals 56 20768 188)
 (buffers 976 128)
 (heap 1024 103678 11159))

bug#25150:

[nitish chandra]

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

The problem seems to be something related to  syntax table usage. If I set
the variable parse-sexp-lookup-properties to nil, motion commands work as
expected.

[-- Attachment #2: Type: text/html, Size: 208 bytes --]

bug#25150: Possible reason for this bug

[nitish chandra]

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

Message mode registers message--syntax-propertize as
syntax-propertize-function. This function is used by FontLock mode to
highlight the buffer.

message--syntax-properties assigns comment-begin class to the ] on the
second line. Because of this, it is ignored in the motion traversal.

But why is this? Why should ] at the start of a line be regarded as a
comment? If > is regarded as a comment, it sounds reasonable because >
characters are used to cite messages. But why ]? Are there any citing
formats that use ] for citing?

Thanks,
Nitish

[-- Attachment #2: Type: text/html, Size: 643 bytes --]

bug#25150: Why is ] included in message-cite-prefix-regexp?

[nitish chandra]

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

This appears to be introduced in the commit [1]. I couldn't find any
explanation of the change of regexp in the previous commit to this commit.

More specifically, in the previous commit, the regexp had [:>|}] character
alternative. But this commit changes it to []>|}+]. Why is ] included in
this character alternative?


[1]
git.savannah.gnu.org/cgit/emacs.git/commit/?id=23f87bede063c31c164f97278caabdc5cf5e6980

Thanks,
Nitish

[-- Attachment #2: Type: text/html, Size: 633 bytes --]

bug#25150: Possible reason for this bug

[npostavs]

nitish chandra <nitishchandrachinta@gmail.com> writes:

>
> But why is this? Why should ] at the start of a line be regarded as a
> comment? If > is regarded as a comment, it sounds reasonable because >
> characters are used to cite messages. But why ]? Are there any citing
> formats that use ] for citing?

https://en.wikipedia.org/wiki/Posting_style#Quoted_line_prefix says

    Today (and for many years previously), the greater-than sign (">",
    the canonical prefix)[1] is almost universally used; but other
    characters such as the ASCII vertical bar character ("|") have been
    used as well, sometimes with one or more spaces inserted before or
    after the quoted text marker.

Perhaps "]" used to be one of the "other" characters used for this?
Seems to be obsolete now, I think it would be sensible to remove it from
the default value of mail-citation-prefix-regexp.

bug#25150: [PATCH] Removing ] from message-cite-prefix-regexp

[nitish chandra]

[-- Attachment #1.1: Type: text/plain, Size: 343 bytes --]

Hi,

Following is the patch to remove ] character from
message-cite-prefix-regexp. Code wise, it just removes the ] character from
the regexp.

I've a question about the code though. Why is the same variable defined in
both lisp/ldefs-boot.el and lisp/mail/sendmail.el? I changed the default
variable value in both the places.

Thanks,
Nitish

[-- Attachment #1.2: Type: text/html, Size: 457 bytes --]

[-- Attachment #2: 0001-Fix-Bug-25150.patch --]
[-- Type: text/x-patch, Size: 1531 bytes --]

From b1b80c8d9b35753c428b655c9b415df78611813e Mon Sep 17 00:00:00 2001
From: nitishch <nitishchandrachinta@gmail.com>
Date: Tue, 20 Dec 2016 00:27:40 +0530
Subject: [PATCH] Fix Bug#25150

---
 lisp/ldefs-boot.el    | 2 +-
 lisp/mail/sendmail.el | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/ldefs-boot.el b/lisp/ldefs-boot.el
index 406f045..3fbfdc6 100644
--- a/lisp/ldefs-boot.el
+++ b/lisp/ldefs-boot.el
@@ -29184,7 +29184,7 @@ mail-citation-hook
 
 (custom-autoload 'mail-citation-hook "sendmail" t)
 
-(defvar mail-citation-prefix-regexp (purecopy "\\([ 	]*\\(\\w\\|[_.]\\)+>+\\|[ 	]*[]>|]\\)+") "\
+(defvar mail-citation-prefix-regexp (purecopy "\\([ 	]*\\(\\w\\|[_.]\\)+>+\\|[ 	]*[>|]\\)+") "\
 Regular expression to match a citation prefix plus whitespace.
 It should match whatever sort of citation prefixes you want to handle,
 with whitespace before and after; it should also match just whitespace.
diff --git a/lisp/mail/sendmail.el b/lisp/mail/sendmail.el
index 3d22209..b8cf632 100644
--- a/lisp/mail/sendmail.el
+++ b/lisp/mail/sendmail.el
@@ -273,7 +273,7 @@ mail-citation-header
 
 ;;;###autoload
 (defcustom mail-citation-prefix-regexp
-  (purecopy "\\([ \t]*\\(\\w\\|[_.]\\)+>+\\|[ \t]*[]>|]\\)+")
+  (purecopy "\\([ \t]*\\(\\w\\|[_.]\\)+>+\\|[ \t]*[>|]\\)+")
   "Regular expression to match a citation prefix plus whitespace.
 It should match whatever sort of citation prefixes you want to handle,
 with whitespace before and after; it should also match just whitespace.
-- 
2.9.0

bug#25150: [PATCH] Removing ] from message-cite-prefix-regexp

[Eli Zaretskii]

> From: nitish chandra <nitishchandrachinta@gmail.com>
> Date: Tue, 20 Dec 2016 00:52:27 +0530
> 
> I've a question about the code though. Why is the same variable defined in both lisp/ldefs-boot.el and
> lisp/mail/sendmail.el? I changed the default variable value in both the places.

Auto-loaded symbols appear in loaddefs.el as well.  ldefs-boot.el is
the version of loaddefs.el needed for bootstrapping Emacs (loaddefs.el
is a generated file, but you need Emacs to generate it).

bug#25150: [PATCH] Removing ] from message-cite-prefix-regexp

[nitish chandra]

[-- Attachment #1.1: Type: text/plain, Size: 671 bytes --]

On 20 Dec 2016 1:14 am, "Eli Zaretskii" <eliz@gnu.org> wrote:
>
> > From: nitish chandra <nitishchandrachinta@gmail.com>
> > Date: Tue, 20 Dec 2016 00:52:27 +0530
> >
> > I've a question about the code though. Why is the same variable defined
in both lisp/ldefs-boot.el and
> > lisp/mail/sendmail.el? I changed the default variable value in both the
places.
>
> Auto-loaded symbols appear in loaddefs.el as well.  ldefs-boot.el is
> the version of loaddefs.el needed for bootstrapping Emacs (loaddefs.el
> is a generated file, but you need Emacs to generate it).

Oh. Cool. Then the code should be changed only in sendmail.el. Following is
the new patch.


Thanks
Nitish

[-- Attachment #1.2: Type: text/html, Size: 997 bytes --]

[-- Attachment #2: 0001-Fix-Bug-25150.patch --]
[-- Type: text/x-patch, Size: 862 bytes --]

From 1de7da4495f2e544cf100515d9c052808d115815 Mon Sep 17 00:00:00 2001
From: nitishch <nitishchandrachinta@gmail.com>
Date: Tue, 20 Dec 2016 10:17:42 +0530
Subject: [PATCH] Fix Bug#25150

---
 lisp/mail/sendmail.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/mail/sendmail.el b/lisp/mail/sendmail.el
index 3d22209..b8cf632 100644
--- a/lisp/mail/sendmail.el
+++ b/lisp/mail/sendmail.el
@@ -273,7 +273,7 @@ mail-citation-header
 
 ;;;###autoload
 (defcustom mail-citation-prefix-regexp
-  (purecopy "\\([ \t]*\\(\\w\\|[_.]\\)+>+\\|[ \t]*[]>|]\\)+")
+  (purecopy "\\([ \t]*\\(\\w\\|[_.]\\)+>+\\|[ \t]*[>|]\\)+")
   "Regular expression to match a citation prefix plus whitespace.
 It should match whatever sort of citation prefixes you want to handle,
 with whitespace before and after; it should also match just whitespace.
-- 
2.9.0

bug#25150: Patch review

[nitish chandra]

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

Any updates/comments on the patch?

Thanks
Nitish

[-- Attachment #2: Type: text/html, Size: 106 bytes --]

bug#25150: [PATCH] Removing ] from message-cite-prefix-regexp

[Lars Ingebrigtsen]

nitish chandra <nitishchandrachinta@gmail.com> writes:

> Oh. Cool. Then the code should be changed only in
> sendmail.el. Following is the new patch.

[...]

>  (defcustom mail-citation-prefix-regexp
> -  (purecopy "\\([ \t]*\\(\\w\\|[_.]\\)+>+\\|[ \t]*[]>|]\\)+")
> +  (purecopy "\\([ \t]*\\(\\w\\|[_.]\\)+>+\\|[ \t]*[>|]\\)+")

I think it makes sense to remove the "]" character from the list of
standard citation prefixes -- it's not standard, and it's not used very
much, either.

So I've applied your patch to Emacs 27.

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