bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Eli Zaretskii]

From: eliz@HOME-C4E4A596F7.i-did-not-set--mail-host-address--so-tickle-me
--text follows this line--
To reproduce:

  emacs -Q
  C-h H

Observe the messed-up alignment of the second column.  The reason is
that the tab-width file-local variable setting didn't take effect;
setting the variable by hand fixes the display.

I suspect this change:

  commit 26171e02773b9b2383f412dd79d241385d2d20df
  Author: Alan Mackenzie <acm@muc.de>
  Date:   Fri May 6 18:58:49 2016 +0000

      Correct hack-local-variables change from Thu May 5 11:05:49 2016 +0000

      Prevent hack-local-variables being called from the fundamental-mode mode call
      early in normal-mode.  This fixes bug #23460 and bug #23463.

      * lisp/files.el (normal-mode) Replace call to fundamental-mode with calls to
      the things it calls, with the exception of hack-local-variables.

      * etc/NEWS: Add an entry to note the calling of hack-local-variables at each
      major mode initialization.


In GNU Emacs 25.1.50.166 (i686-pc-mingw32)
 of 2016-06-22 built on HOME-C4E4A596F7
Repository revision: cc113e557d56d849e9699ceb3bc4a735c628b46e
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --prefix=/d/usr --enable-checking=yes,glyphs --with-wide-int
 --with-modules 'CFLAGS=-O0 -gdwarf-4 -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS MODULES

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  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

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message puny seq byte-opt gv bytecomp
byte-compile cl-extra help-mode cconv cl-loaddefs pcase cl-lib dired
dired-loaddefs format-spec rfc822 mml easymenu 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
time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win
w32-vars term/common-win 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 w32notify w32 multi-tty
make-network-process emacs)

Memory information:
((conses 16 102815 10519)
 (symbols 56 21684 0)
 (miscs 48 36 108)
 (strings 16 20874 5712)
 (string-bytes 1 586184)
 (vectors 16 14055)
 (vector-slots 8 442248 5351)
 (floats 8 181 48)
 (intervals 40 268 99)
 (buffers 856 11))

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Alan Mackenzie]

Hello, Eli.

In article <mailman.2018.1466615229.1216.bug-gnu-emacs@gnu.org> you wrote:
> From: eliz@HOME-C4E4A596F7.i-did-not-set--mail-host-address--so-tickle-me
> --text follows this line--
> To reproduce:

>   emacs -Q
>   C-h H

> Observe the messed-up alignment of the second column.  The reason is
> that the tab-width file-local variable setting didn't take effect;
> setting the variable by hand fixes the display.

> I suspect this change:

>   commit 26171e02773b9b2383f412dd79d241385d2d20df
>   Author: Alan Mackenzie <acm@muc.de>
>   Date:   Fri May 6 18:58:49 2016 +0000

>       Correct hack-local-variables change from Thu May 5 11:05:49 2016 +0000

>       Prevent hack-local-variables being called from the fundamental-mode mode call
>       early in normal-mode.  This fixes bug #23460 and bug #23463.

>       * lisp/files.el (normal-mode) Replace call to fundamental-mode with calls to
>       the things it calls, with the exception of hack-local-variables.

>       * etc/NEWS: Add an entry to note the calling of hack-local-variables at each
>       major mode initialization.

Yes.  That change (and the change it corrected) shifted the call of
`hack-local-variables' from the act of visiting a file, to the calling of
the major mode function.  Every major mode (including fundamental-mode)
calls `run-mode-hooks' which calls `hack-local-variables'.

However, the C function `set-buffer-major-mode', optimises the call to
`fundamental-mode' away, because that call previously didn't do anything.

(`set-buffer-major-mode' is the last fallback function which choses the
major mode when all other methods have been tried and failed in
`set-auto-mode'.)

So I propose to remove that special optimisation from
`set-buffer-major-mode', so that `fundamental-mode' actually gets called.
As an alternative, it would be possible to add special handling at the
Lisp level for `fundamental-mode', but I think that would be a worse fix.

Here's my proposed patch, which works:


diff --git a/src/buffer.c b/src/buffer.c
index b4b8304..8756cbb 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1984,7 +1984,9 @@ the current buffer's major mode.  */)
 	function = BVAR (current_buffer, major_mode);
     }
 
-  if (NILP (function) || EQ (function, Qfundamental_mode))
+  if (NILP (function)) /* If function is `fundamental-mode', allow it to run
+                          so that `run-mode-hooks' and thus
+                          `hack-local-variables' get run. */
     return Qnil;
 
   count = SPECPDL_INDEX ();


What do you think?


> In GNU Emacs 25.1.50.166 (i686-pc-mingw32)
>  of 2016-06-22 built on HOME-C4E4A596F7
> Repository revision: cc113e557d56d849e9699ceb3bc4a735c628b46e
> Windowing system distributor 'Microsoft Corp.', version 5.1.2600
> Recent messages:
> For information about GNU Emacs and the GNU system, type C-h C-a.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Eli Zaretskii]

> Date: 24 Jun 2016 11:07:13 -0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> However, the C function `set-buffer-major-mode', optimises the call to
> `fundamental-mode' away, because that call previously didn't do anything.
> 
> (`set-buffer-major-mode' is the last fallback function which choses the
> major mode when all other methods have been tried and failed in
> `set-auto-mode'.)
> 
> So I propose to remove that special optimisation from
> `set-buffer-major-mode', so that `fundamental-mode' actually gets called.
> As an alternative, it would be possible to add special handling at the
> Lisp level for `fundamental-mode', but I think that would be a worse fix.
> 
> Here's my proposed patch, which works:
> 
> 
> diff --git a/src/buffer.c b/src/buffer.c
> index b4b8304..8756cbb 100644
> --- a/src/buffer.c
> +++ b/src/buffer.c
> @@ -1984,7 +1984,9 @@ the current buffer's major mode.  */)
>  	function = BVAR (current_buffer, major_mode);
>      }
>  
> -  if (NILP (function) || EQ (function, Qfundamental_mode))
> +  if (NILP (function)) /* If function is `fundamental-mode', allow it to run
> +                          so that `run-mode-hooks' and thus
> +                          `hack-local-variables' get run. */
>      return Qnil;
>  
>    count = SPECPDL_INDEX ();
> 
> 
> What do you think?

LGTM, but I'm not an expert on modes.  I'd like to hear opinions from
others.  Stefan?

Thanks.

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> LGTM, but I'm not an expert on modes.  I'd like to hear opinions from
> others.  Stefan?

set-buffer-major-mode and the question about when it's supposed to be
run (and when not), and what it's supposed to do (and how that
interacts with default-major-mode) is still pretty nebulous to
me, sorry.

I tend to agree that it "looks right", so we should probably just
install it into master and see what breaks.  My only real worry is
whether that would end up calling fundamental-mode when creating
temporary buffers such as in `with-temp-buffer' (where we wouldn't want
to run-mode-hooks and hack-local-variables).


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Alan Mackenzie <acm@muc.de>, 23827@debbugs.gnu.org
> Date: Fri, 24 Jun 2016 10:12:10 -0400
> 
> I tend to agree that it "looks right", so we should probably just
> install it into master and see what breaks.  My only real worry is
> whether that would end up calling fundamental-mode when creating
> temporary buffers such as in `with-temp-buffer' (where we wouldn't want
> to run-mode-hooks and hack-local-variables).

So you suggest to have a variable whose value could bypass calling
fundamental-mode, and bind that variable inside with-temp-buffer and
such likes?

Is there another alternative?

Thanks.

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> So you suggest to have a variable whose value could bypass calling
> fundamental-mode, and bind that variable inside with-temp-buffer and
> such likes?

Actually, not really, I was rather asking whether this change would
cause with-temp-buffer to run hack-local-variables.

If it does, then I think that's an undesirable side-effect and we
should try and find some way around it


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Alan Mackenzie]

Hello, Stefan and Eli.

On Fri, Jun 24, 2016 at 10:27:06AM -0400, Stefan Monnier wrote:
> > So you suggest to have a variable whose value could bypass calling
> > fundamental-mode, and bind that variable inside with-temp-buffer and
> > such likes?

> Actually, not really, I was rather asking whether this change would
> cause with-temp-buffer to run hack-local-variables.

I've installed the patch into master.

I rather think a buffer made by `with-temp-buffer' will run
`hack-local-variables', but since there will never be a local variables
section in the new buffer, what are we worrying about?

> If it does, then I think that's an undesirable side-effect and we
> should try and find some way around it

Why?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Leo Liu]

On 2016-06-24 16:09 +0000, Alan Mackenzie wrote:
> I rather think a buffer made by `with-temp-buffer' will run
> `hack-local-variables', but since there will never be a local variables
> section in the new buffer, what are we worrying about?

Did you forget dir-local variables?

Leo

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Alan Mackenzie]

Hello, Leo.

In article <mailman.2101.1466786228.1216.bug-gnu-emacs@gnu.org> you wrote:
> On 2016-06-24 16:09 +0000, Alan Mackenzie wrote:
>> I rather think a buffer made by `with-temp-buffer' will run
>> `hack-local-variables', but since there will never be a local variables
>> section in the new buffer, what are we worrying about?

> Did you forget dir-local variables?

I rather think I did.  ;-)  But remembering them now, and looking them up
in the manual, dir-local variables are only applied to file buffers when
they are in the pertinent directory.  A `with-temp-buffer' buffer
shouldn't be in any directory, hence shouldn't get any dir-locals.

That's the theory, anyway.

And even if a temp-buffer did somehow belong to a directory, what harm
will it do if it gets the `fundamental-mode' variables?  Probably not
much.

> Leo

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> I rather think a buffer made by `with-temp-buffer' will run
> `hack-local-variables', but since there will never be a local variables
> section in the new buffer, what are we worrying about?

Why such a hurry?


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Alan Mackenzie]

Hello, Stefan.

On Fri, Jun 24, 2016 at 08:21:35PM -0400, Stefan Monnier wrote:
> > I rather think a buffer made by `with-temp-buffer' will run
> > `hack-local-variables', but since there will never be a local variables
> > section in the new buffer, what are we worrying about?

> Why such a hurry?

Well, it seemed the discussion was over, and it felt like the sort of
bug which was annoying somebody.  So, why hang around?

If you've had any more concerns about it, I could always revert it for
the time being.  Have you had any more such concerns?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> If you've had any more concerns about it, I could always revert it for
> the time being.  Have you had any more such concerns?

I still have the question whether with-temp-buffer is affected.
If it is, yes, that's a real concern.  If not, the patch is fine.


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Alan Mackenzie]

Hello, Stefan.

On Mon, Jun 27, 2016 at 06:03:50PM -0400, Stefan Monnier wrote:
> > If you've had any more concerns about it, I could always revert it for
> > the time being.  Have you had any more such concerns?

> I still have the question whether with-temp-buffer is affected.
> If it is, yes, that's a real concern.  If not, the patch is fine.

I think the temp-buffer will pick up any .dir-locals from its "current
directory", where the current directory is merely that of the buffer
which was current when the temp-buffer was created.

I wish I didn't have to say that.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> I think the temp-buffer will pick up any .dir-locals from its "current

There's no need to think/guess here, really.  Instead someone needs to
look at the code and/or test it.


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Alan Mackenzie]

Hello, Stefan.

On Tue, Jun 28, 2016 at 05:43:42PM -0400, Stefan Monnier wrote:
> > I think the temp-buffer will pick up any .dir-locals from its "current

> There's no need to think/guess here, really.  Instead someone needs to
> look at the code and/or test it.

Oh, OK.

I put the following into my .dir-locals.el:

(fundamental-mode . ((indent-tabs-mode . nil)))

, and then ran the following command "in" the current directory
containing the .dir-locals.el:

  M-: (with-temp-buffer (message "indent-tabs-mode: %s.  major-mode: %s" indent-tabs-mode major-mode))

Displayed was:

  "indent-tabs-mode: t.  major-mode: fundamental-mode"

.  So it would appear that the temporary buffer, although ostensibly in
fundamental mode, isn't picking up the .dir-local.el variables.  In fact,
it would appear, from looking at the code, that `fundamental-mode' doesn't
get called.  It isn't clear to me where `major-mode', which presumably is
set to binary zeros when the buffer structure gets allocated, gets set to
`fundamental-mode'.

Maybe we just don't need to worry about this - it appears to do the Right
Thing.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> I put the following into my .dir-locals.el:
> (fundamental-mode . ((indent-tabs-mode . nil)))

A more reliable check would have been to add a function to
hack0local-variables-hook.

> Maybe we just don't need to worry about this - it appears to do the Right
> Thing.

Great, thanks,


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Stefan Monnier]

> get called.  It isn't clear to me where `major-mode', which presumably is
> set to binary zeros when the buffer structure gets allocated, gets set to
> `fundamental-mode'.

"grep -n fundamental_mode src/*.c" should give you the answer ;-)


        Stefan

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 30 Jun 2016 18:39:11 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, 23827@debbugs.gnu.org
> 
> > get called.  It isn't clear to me where `major-mode', which presumably is
> > set to binary zeros when the buffer structure gets allocated, gets set to
> > `fundamental-mode'.
> 
> "grep -n fundamental_mode src/*.c" should give you the answer ;-)

This bug should closed, right?  At least I no longer see the original
problem on master.

bug#23827: 25.1.50; tab-width file-local variable has no effect in etc/HELLO

[Eli Zaretskii]

> Date: Mon, 10 Oct 2016 13:37:16 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: acm@muc.de, 23827@debbugs.gnu.org
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Date: Thu, 30 Jun 2016 18:39:11 -0400
> > Cc: Eli Zaretskii <eliz@gnu.org>, 23827@debbugs.gnu.org
> > 
> > > get called.  It isn't clear to me where `major-mode', which presumably is
> > > set to binary zeros when the buffer structure gets allocated, gets set to
> > > `fundamental-mode'.
> > 
> > "grep -n fundamental_mode src/*.c" should give you the answer ;-)
> 
> This bug should closed, right?  At least I no longer see the original
> problem on master.

No further comments, so I'm marking this bug done.

Thanks.