bug#23460: 25.1.50; Strange behavior in local variables like start-comment

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Kaushal Modi]

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

--text follows this line--

It looks like after the recent commit (
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=25f455815bfaa868dc470d445413df9a7a546c46
),
the buffer local variables like comment-start are behaving strangely. They
commentarily go to a value of nil (or their default value?) and then back
to the buffer local value in the current buffer.

I discovered this issue as I am using outshine package and in one of the
functions in there, a check is put to ensure that comment-start is not nil:
https://github.com/tj64/outshine/blob/73ae2c2ea24001338dbeff5137f09bb5f784f12c/outshine.el#L1105-L1108

Surprisingly, my *Messages* buffer started showing the "Cannot calculate
outcommented outline-regexp without 'comment-start' character defined!"
message. But I was working in a shell-script-mode buffer and comment-start
IS defined: "# ". I even verified doing C-h v comment-start that that
actually was the case.

I then realized that I saw that message each time I reverted the buffer (or
looked like each time the buffer's major mode hook was called (and that's
why I suspect commit 25f4558.. to be the problem; please correct if that's
wrong). I do not see this bug in emacs-25 branch build.

Here is a minimum working example to test this bug behavior in an emacs -Q
session.

(progn
  (defun my/print-comment-start-value ()
    (interactive)
    (let ((filler (if (null comment-start)
      "is undefined, it's value is:"
    "=")))
      (message (format "`comment-start' %s `%s'" filler comment-start))))

    (global-set-key (kbd "<f5>") (lambda () (interactive) (revert-buffer
nil :noconfirm)))
    (global-set-key (kbd "<f6>") #'my/print-comment-start-value)

    (add-hook 'change-major-mode-hook #'my/print-comment-start-value))

(1) Evaluate the above in emacs -Q
(2) You would be in *scratch* buffer and that's alright
(3) Hit F5 couple of times and you should see
     ‘comment-start’ is undefined, it’s value is: ‘nil’
(4) Now hit F6 (which just prints the value of comment-start variable) and
you will see
    ‘comment-start’ = ‘;’
(5) Isn't that strange?!

I have tested this only for comment-start. But I suspect this might affect
buffer local vars in general?

In GNU Emacs 25.1.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.23)
 of 2016-05-05 built on ...
Repository revision: 67fa7f13d499eb5fc1d697da6c636b20728da22f
Windowing system distributor 'The X.Org Foundation', version 11.0.60900000
System Description: Red Hat Enterprise Linux Workstation release 6.6
(Santiago)

Configured using:
 'configure --with-modules
 --prefix=/home/kmodi/usr_local/apps/6/emacs/master
 'CPPFLAGS=-fgnu89-inline -I/home/kmodi/usr_local/6/include
 -I/usr/include/freetype2 -I/usr/include' 'CFLAGS=-O2 -march=native'
 'LDFLAGS=-L/home/kmodi/usr_local/6/lib -L/home/kmodi/usr_local/6/lib64'
 PKG_CONFIG_PATH=/home/kmodi/usr_local/6/lib/pkgconfig:/home/kmodi/usr_local/6/lib64/pkgconfig:/cad/adi/apps/gnu/linux/x86_64/6/lib/pkgconfig:/cad/adi/apps/gnu/linux/x86_64/6/lib64/pkgconfig:/usr/lib/pkgconfig:/usr/lib64/pkgconfig:/usr/share/pkgconfig:/lib/pkgconfig:/lib64/pkgconfig'

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

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

Major mode: Emacs-Lisp

Minor modes in effect:
  diff-auto-refine-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-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
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
‘comment-start’ is undefined, it’s value is: ‘nil’
‘comment-start’ = ‘;’
‘comment-start’ is undefined, it’s value is: ‘nil’
‘comment-start’ = ‘;’
‘comment-start’ is undefined, it’s value is: ‘nil’
‘comment-start’ = ‘;’ [2 times]
‘comment-start’ is undefined, it’s value is: ‘nil’
‘comment-start’ = ‘;’ [4 times]
‘comment-start’ is undefined, it’s value is: ‘nil’
Mark set [2 times]

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message puny seq byte-opt gv bytecomp
byte-compile cl-extra cconv dired dired-loaddefs format-spec rfc822 mml
mml-sec password-cache epa derived epg epg-config gnus-util rmail
rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils edmacro kmacro dabbrev thingatpt help-fns
help-mode cl-loaddefs pcase cl-lib misearch multi-isearch vc-git
diff-mode easymenu easy-mmode 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 move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 106397 10580)
 (symbols 48 21437 0)
 (miscs 40 69 299)
 (strings 32 20571 5469)
 (string-bytes 1 636526)
 (vectors 16 14269)
 (vector-slots 8 449017 7199)
 (floats 8 192 484)
 (intervals 56 1492 352)
 (buffers 976 14)
 (heap 1024 17132 609))




-- 

-- 
Kaushal Modi

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

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Kaushal Modi]

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

I can definitely see that something about local vars is broken. But please
ignore that earlier min working example for time being. It breaks on
emacs-25 build too.

All I know for sure is that the comment-start being undefined error never
showed up on emacs-25 builds, and it showed up for the first time on the
latest master build as of today.

So I will try to derive a min working example by studying the outshine.el
code. But in the mean time, if you can quickly review why comment-start
could be in undefined state momentarily, that would be great!

I'll keep you posted.

Thanks.
-- 

-- 
Kaushal Modi

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

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Kaushal Modi]

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

>
>
> I'll keep you posted.
>
>
OK, I was able to get a minimum working example sooner than I thought :)

Please save the below as test.el (or some file name; else revert-buffer
will not work), open it in emacs -Q and evaluate it.

===== test.el =====
(progn
  (defun my/print-comment-start-value ()
    (interactive)
    (message (format "`comment-start' = `%s'" comment-start)))

  (defun my/repeat-revert-buffer-noconfirm (count)
    (interactive "p")
    (dotimes (i count)
      (revert-buffer nil :noconfirm)))

  (add-hook 'outline-minor-mode-hook #'my/print-comment-start-value)
;  (remove-hook 'outline-minor-mode-hook #'my/print-comment-start-value)

  (global-set-key (kbd "<f5>") #'my/repeat-revert-buffer-noconfirm))

;; Local Variables:
;; eval: (outline-minor-mode 1)
;; End:
=====

In emacs 25.0.93 (emacs-25 build), after opening test.el, evaluating it and
hitting F5, I get below (which is good, expected):

‘comment-start’ = ‘;’ [2 times]

But in emacs 25.1.50 (master build), after opening test.el, evaluating it
and hitting F5, I get:

‘comment-start’ = ‘;’
‘comment-start’ = ‘nil’ [2 times]
‘comment-start’ = ‘;’

So it looks like the outline-minor-mode-hook is called twice more in the
master branch and also intermittently, the comment-start loses its buffer
local value.

Hope this mini test helps debug this.

> --

-- 
Kaushal Modi

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

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Alan Mackenzie]

Hello, Kaushal.

On Thu, May 05, 2016 at 10:20:27PM +0000, Kaushal Modi wrote:


> > I'll keep you posted.


> OK, I was able to get a minimum working example sooner than I thought :)

> Please save the below as test.el (or some file name; else revert-buffer
> will not work), open it in emacs -Q and evaluate it.

> ===== test.el =====
> (progn
>   (defun my/print-comment-start-value ()
>     (interactive)
>     (message (format "`comment-start' = `%s'" comment-start)))

>   (defun my/repeat-revert-buffer-noconfirm (count)
>     (interactive "p")
>     (dotimes (i count)
>       (revert-buffer nil :noconfirm)))

>   (add-hook 'outline-minor-mode-hook #'my/print-comment-start-value)
> ;  (remove-hook 'outline-minor-mode-hook #'my/print-comment-start-value)

>   (global-set-key (kbd "<f5>") #'my/repeat-revert-buffer-noconfirm))

> ;; Local Variables:
> ;; eval: (outline-minor-mode 1)
> ;; End:
> =====

> In emacs 25.0.93 (emacs-25 build), after opening test.el, evaluating it and
> hitting F5, I get below (which is good, expected):

> ‘comment-start’ = ‘;’ [2 times]

> But in emacs 25.1.50 (master build), after opening test.el, evaluating it
> and hitting F5, I get:

> ‘comment-start’ = ‘;’
> ‘comment-start’ = ‘nil’ [2 times]
> ‘comment-start’ = ‘;’

> So it looks like the outline-minor-mode-hook is called twice more in the
> master branch and also intermittently, the comment-start loses its buffer
> local value.

> Hope this mini test helps debug this.

Yes, thanks!

I think I've nailed it - normal-mode is calling hack-local-variables
right at the start, when it shouldn't.

Could you please try out the following patch (which I've also posted for
bug #23463):



diff --git a/lisp/files.el b/lisp/files.el
index d89b2f5..21f7bdc 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2316,7 +2316,10 @@ normal-mode
 or from Lisp without specifying the optional argument FIND-FILE;
 in that case, this function acts as if `enable-local-variables' were t."
   (interactive)
-  (fundamental-mode)
+  (kill-all-local-variables)
+  (unless delay-mode-hooks
+    (run-hooks 'change-major-mode-after-body-hook
+               'after-change-major-mode-hook))
   (let ((enable-local-variables (or (not find-file) enable-local-variables)))
     ;; FIXME this is less efficient than it could be, since both
     ;; s-a-m and h-l-v may parse the same regions, looking for "mode:".


> -- 
> Kaushal Modi

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Kaushal Modi]

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

On Fri, May 6, 2016 at 5:37 AM Alan Mackenzie <acm@muc.de> wrote:

> Yes, thanks!
>
> I think I've nailed it - normal-mode is calling hack-local-variables
> right at the start, when it shouldn't.
>
> Could you please try out the following patch (which I've also posted for
> bug #23463):
>

Thanks! That fixes the bug for me; I tested it both on that MWE and running
the master build with fixed files.el using my full-blown config :)
-- 

-- 
Kaushal Modi

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

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Alan Mackenzie]

Hello, Kaushal.

On Thu, May 05, 2016 at 08:35:04PM +0000, Kaushal Modi wrote:
> It looks like after the recent commit (
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=25f455815bfaa868dc470d445413df9a7a546c46
> ),
> the buffer local variables like comment-start are behaving strangely. They
> commentarily go to a value of nil (or their default value?) and then back
> to the buffer local value in the current buffer.

Yes.  I think this is intended, and wasn't changed by my recent patch.

> I discovered this issue as I am using outshine package and in one of the
> functions in there, a check is put to ensure that comment-start is not nil:
> https://github.com/tj64/outshine/blob/73ae2c2ea24001338dbeff5137f09bb5f784f12c/outshine.el#L1105-L1108

I haven't actually looked at that yet.  But where, with respect to
setting the major mode, does the outshine function look at
comment-start?

> Surprisingly, my *Messages* buffer started showing the "Cannot calculate
> outcommented outline-regexp without 'comment-start' character defined!"
> message. But I was working in a shell-script-mode buffer and comment-start
> IS defined: "# ". I even verified doing C-h v comment-start that that
> actually was the case.

Were you doing anything involving changing the major mode at the time?
(See below.)

> I then realized that I saw that message each time I reverted the buffer (or
> looked like each time the buffer's major mode hook was called (and that's
> why I suspect commit 25f4558.. to be the problem; please correct if that's
> wrong). I do not see this bug in emacs-25 branch build.

> Here is a minimum working example to test this bug behavior in an emacs -Q
> session.

Thanks for taking the trouble to create this minimal test code.  It
makes it so much easier to understand when people do this.  :-)

> (progn
>   (defun my/print-comment-start-value ()
>     (interactive)
>     (let ((filler (if (null comment-start)
>       "is undefined, it's value is:"
>     "=")))
>       (message (format "`comment-start' %s `%s'" filler comment-start))))

>     (global-set-key (kbd "<f5>") (lambda () (interactive) (revert-buffer
> nil :noconfirm)))
>     (global-set-key (kbd "<f6>") #'my/print-comment-start-value)

>     (add-hook 'change-major-mode-hook #'my/print-comment-start-value))

> (1) Evaluate the above in emacs -Q
> (2) You would be in *scratch* buffer and that's alright
> (3) Hit F5 couple of times and you should see
>      ‘comment-start’ is undefined, it’s value is: ‘nil’
> (4) Now hit F6 (which just prints the value of comment-start variable) and
> you will see
>     ‘comment-start’ = ‘;’
> (5) Isn't that strange?!

OK.  Have a look at the *Messages* buffer.  You will see the messages
come in pairs:  (I actually did this in a C Mode buffer.)

    `comment-start' = `/* '
    `comment-start' is undefined, it's value is: `nil'

But in the echo area, you will only see the second one.

> I have tested this only for comment-start. But I suspect this might affect
> buffer local vars in general?

I think so, too.  When you visit a buffer, Emacs calls `normal-mode' to
determine and set the major mode.  The first thing `normal-mode' does is
to call `fundamental-mode'.  This invokes `change-major-mode-hook',
which outputs the first "comment-start" message.  Then
`fundamental-mode' kills all local variables, including `comment-start'.

`normal-mode' then determines the correct major mode, and calls it.
This invokes `change-major-mode-hook' again, which outputs the second
"comment-start" message with the `nil'.  The major mode is then fully
set up, including giving a value to `comment-start'.

So, this code seems to be working the way it's intended to.  What is
outshine doing that triggers a problem in this area?  Is it doing things
with the `change-major-mode-hook' that involve using `comment-start'?
Maybe this is the problem.

> In GNU Emacs 25.1.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.23)
>  of 2016-05-05 built on ...
> Repository revision: 67fa7f13d499eb5fc1d697da6c636b20728da22f
> Windowing system distributor 'The X.Org Foundation', version 11.0.60900000
> System Description: Red Hat Enterprise Linux Workstation release 6.6
> (Santiago)

[ .... ]

> Recent messages:
> ‘comment-start’ is undefined, it’s value is: ‘nil’
> ‘comment-start’ = ‘;’
> ‘comment-start’ is undefined, it’s value is: ‘nil’
> ‘comment-start’ = ‘;’
> ‘comment-start’ is undefined, it’s value is: ‘nil’
> ‘comment-start’ = ‘;’ [2 times]
> ‘comment-start’ is undefined, it’s value is: ‘nil’
> ‘comment-start’ = ‘;’ [4 times]
> ‘comment-start’ is undefined, it’s value is: ‘nil’
> Mark set [2 times]

> -- 
> Kaushal Modi

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Kaushal Modi]

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

>
> I haven't actually looked at that yet.  But where, with respect to
> setting the major mode, does the outshine function look at
> comment-start?
>

Yes


> Were you doing anything involving changing the major mode at the time?
> (See below.)
>

Apologies for that misleading first version of the MWE; the issue I am
seeing has nothing to do with changing major mode (I think).

Thanks for taking the trouble to create this minimal test code.  It
> makes it so much easier to understand when people do this.  :-)
>

No worries. But unintentional I caused more trouble for you by posting an
incorrect version of MWE (the 1st version). The second version I posted few
minutes back is thoroughly tested in both emacs-25 and master and has no
outshine.el dependency i.e. you can focus only on that 2nd version of MWE.

OK.  Have a look at the *Messages* buffer.  You will see the messages
> come in pairs:  (I actually did this in a C Mode buffer.)
>
>     `comment-start' = `/* '
>     `comment-start' is undefined, it's value is: `nil'
>
> But in the echo area, you will only see the second one.
>
> > I have tested this only for comment-start. But I suspect this might
> affect
> > buffer local vars in general?
>
> I think so, too.  When you visit a buffer, Emacs calls `normal-mode' to
> determine and set the major mode.  The first thing `normal-mode' does is
> to call `fundamental-mode'.  This invokes `change-major-mode-hook',
> which outputs the first "comment-start" message.  Then
> `fundamental-mode' kills all local variables, including `comment-start'.
>
> `normal-mode' then determines the correct major mode, and calls it.
> This invokes `change-major-mode-hook' again, which outputs the second
> "comment-start" message with the `nil'.  The major mode is then fully
> set up, including giving a value to `comment-start'.
>
> So, this code seems to be working the way it's intended to.


Thank you for that detailed explanation. I was not aware of the major mode
changes that were happening behind the scenes.


> What is
> outshine doing that triggers a problem in this area?


The outshine-mode-function function is actually added to
outline-minor-mode-hook. I have updated MWE version 2 with that in mind.

Is it doing things
> with the `change-major-mode-hook' that involve using `comment-start'?
> Maybe this is the problem.
>

No, it does not use change-major-mode-hook. But it does depend on the
buffer local value of comment-start.

In summary, the 2nd version of my MWE should help you consistently show the
behavior difference between emacs-25 and master.
-- 

-- 
Kaushal Modi

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

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Michael Heerdegen]

Kaushal Modi <kaushal.modi@gmail.com> writes:

> (1) Evaluate the above in emacs -Q
> (2) You would be in *scratch* buffer and that's alright
> (3) Hit F5 couple of times and you should see
> ‘comment-start’ is undefined, it’s value is: ‘nil’
> (4) Now hit F6 (which just prints the value of comment-start variable) and you will see
> ‘comment-start’ = ‘;’
> (5) Isn't that strange?!

Strange, but, unless I misunderstood what your described, expected:

Reverting is a typical case where all local variable definitions are
cleared (intentionally): Unless arg PRESERVE-MODES is non-nil, this
calls `after-find-file' which calls `normal-mode' that (re)sets the
major mode of the buffer.  Changing the major mode typically always
kills all buffer local bindings.

So to say, it's a feature: `revert-buffer' also reverts local variable
bindings.


Michael.

bug#23460: 25.1.50; Strange behavior in local variables like start-comment

[Alan Mackenzie]

Bug fixed in the master branch.

-- 
Alan Mackenzie (Nuremberg, Germany).