bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Robin Neatherway]

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

Run `emacs -Q -l tmp.el` (major mode tmp defined in attached file
tmp.el).
Type `hello RET`, observe "Called indent-line!" in minibuffer.
Type `/* hello RET`, observe no message in minibuffer.



In GNU Emacs 24.4.1 (x86_64-pc-linux-gnu, GTK+ Version 3.14.5)
 of 2015-03-07 on trouble, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11604000
System Description:    Debian GNU/Linux 8.0 (jessie)

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/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/site-lisp
 --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/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/site-lisp
 --with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
 'CFLAGS=-g -O2 -fstack-protector-strong -Wformat
 -Werror=format-security -Wall' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-z,relro'

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

Major mode: Tmp

Minor modes in effect:
  tooltip-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 input:
<help-echo> C-x C-f t e s t . t m p <return> / / SPC
h e l l o <return> <return> / * SPC h e l l o SPC <return>
C-x C-s <down-mouse-1> <mouse-1> M-x r e p o r t <tab>
<return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
(New file)
Called indent-line! [4 times]
Saving file /home/robin/dev/other/test.tmp...
Wrote /home/robin/dev/other/test.tmp

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils tmp generic time-date tooltip
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode prog-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer 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 make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 71973 4467)
 (symbols 48 17768 0)
 (miscs 40 46 138)
 (strings 32 9223 3921)
 (string-bytes 1 254557)
 (vectors 16 9026)
 (vector-slots 8 388578 12867)
 (floats 8 63 274)
 (intervals 56 222 4)
 (buffers 960 12)
 (heap 1024 47744 997))

[-- Attachment #2: tmp.el --]
[-- Type: text/x-emacs-lisp, Size: 764 bytes --]

(define-generic-mode
    tmp-mode ; mode name

  ;; comments
  nil

  ;; keywords
  '("if" "then" "else")

  ;; other things to highlight
  nil

  ;; auto mode alist
  '("\\.tmp?$")

  ;; other function to run
  (list 'tmp-mode--setup-function)
  "A mode for tmp files")

(defun tmp-mode--setup-function ()
  (modify-syntax-entry ?_ "w" (syntax-table))
  ;; // Comments (style a)
  (modify-syntax-entry ?\/ ". 124" (syntax-table))
  (modify-syntax-entry ?\n "> " (syntax-table))
  ;; /* Comments (style b) */
  (modify-syntax-entry ?* ". 23b" (syntax-table))
  (modify-syntax-entry ?@ "_" (syntax-table))

  (set (make-local-variable 'indent-line-function) 'tmp-mode/indent-line))

(defun tmp-mode/indent-line ()
  (message "Called indent-line!"))

(provide 'tmp)

bug#20846: Reproduction instruction correction

[Robin Neatherway]

It is necessary to first open a file with the .tmp extension or
otherwise ensure that the 'tmp' major mode is activated.

E.g: `C-x C-f test.tmp RET`

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Lars Ingebrigtsen]

Robin Neatherway <robin.neatherway@gmail.com> writes:

> Run `emacs -Q -l tmp.el` (major mode tmp defined in attached file
> tmp.el).
> Type `hello RET`, observe "Called indent-line!" in minibuffer.
> Type `/* hello RET`, observe no message in minibuffer.

(I'm going through old bug reports that unfortunately got no response at
the time.)

If I understand correctly, this bug report is about
`indent-line-function' not being called in comments?

In which case, this if an easier way to reproduce is:

emacs -Q /tmp/foo.c
M-: (setq-local indent-line-function (lambda () (message "indent"))) RET
/* RET

Inside the comment, the indentation function is not called, but it's
called everywhere else.

I'm not quite sure whether this is by design or not -- TAB is certainly
meaningful in comments, so I'm not sure why RET is handled specially
here.

That is, with 

/* Yes
no

will indent to

/* Yes
   no

but RET doesn't move point the same way.  Which seems inconsistent.

Anybody have an opinion here?

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

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 27 May 2021 01:32:50 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 20846@debbugs.gnu.org
> 
> If I understand correctly, this bug report is about
> `indent-line-function' not being called in comments?
> 
> In which case, this if an easier way to reproduce is:
> 
> emacs -Q /tmp/foo.c
> M-: (setq-local indent-line-function (lambda () (message "indent"))) RET
> /* RET
> 
> Inside the comment, the indentation function is not called, but it's
> called everywhere else.
> 
> I'm not quite sure whether this is by design or not -- TAB is certainly
> meaningful in comments, so I'm not sure why RET is handled specially
> here.

maybe the idea is that indent-line-function is only for indenting
code?

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> maybe the idea is that indent-line-function is only for indenting
> code?

The function is called when you TAB inside a comment, though.

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

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: robin.neatherway@gmail.com,  monnier@iro.umontreal.ca,
>   20846@debbugs.gnu.org
> Date: Fri, 28 May 2021 01:43:11 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > maybe the idea is that indent-line-function is only for indenting
> > code?
> 
> The function is called when you TAB inside a comment, though.

And the doc string says, inter alia:

  If it is called somewhere where auto-indentation cannot be done
  (e.g. inside a string), the function should simply return ‘noindent’.
  ^^^^^^^^^^^^^^^^^^^^^^

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Dmitry Gutov]

On 28.05.2021 08:49, Eli Zaretskii wrote:
>    If it is called somewhere where auto-indentation cannot be done
>    (e.g. inside a string), the function should simply return ‘noindent’.

I believe it's the choice of the indentation function - whether it works 
inside string, comments, etc. Some do, some don't.

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Eli Zaretskii]

> Cc: robin.neatherway@gmail.com, monnier@iro.umontreal.ca,
>  20846@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 28 May 2021 15:56:51 +0300
> 
> On 28.05.2021 08:49, Eli Zaretskii wrote:
> >    If it is called somewhere where auto-indentation cannot be done
> >    (e.g. inside a string), the function should simply return ‘noindent’.
> 
> I believe it's the choice of the indentation function - whether it works 
> inside string, comments, etc. Some do, some don't.

Then that sentence is misleading and should be rephrased, because my
reading of it is that it clearly states that auto-indentation inside
strings is generally impossible, and expects the implementations to
behave like that.

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Dmitry Gutov]

On 28.05.2021 16:06, Eli Zaretskii wrote:
> Then that sentence is misleading and should be rephrased,

Maybe? I don't have a better wording to suggest off the top of my head.

> because my
> reading of it is that it clearly states that auto-indentation inside
> strings is generally impossible, and expects the implementations to
> behave like that.

Consider this from a design POV: if all indentation functions were 
supposed to return 'noindent when inside a string or comment, 
indent-for-tab-command could do that check itself and avoid calling 
indent-line-function entirely in that case.

And also that #'indent-relative (the default value of 
indent-line-function) does indent inside strings and comments.

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Eli Zaretskii]

> Cc: robin.neatherway@gmail.com, larsi@gnus.org, monnier@iro.umontreal.ca,
>  20846@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 28 May 2021 16:35:51 +0300
> 
> > because my
> > reading of it is that it clearly states that auto-indentation inside
> > strings is generally impossible, and expects the implementations to
> > behave like that.
> 
> Consider this from a design POV: if all indentation functions were 
> supposed to return 'noindent when inside a string or comment, 
> indent-for-tab-command could do that check itself and avoid calling 
> indent-line-function entirely in that case.

Not if calling the function in that case was a rare exception, which
is what my interpretation of that text suggests.

> And also that #'indent-relative (the default value of 
> indent-line-function) does indent inside strings and comments.

Documentation should be clear enough to not require the reader to
consider examples in order to understand it.

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Then that sentence is misleading and should be rephrased, because my
> reading of it is that it clearly states that auto-indentation inside
> strings is generally impossible, and expects the implementations to
> behave like that.

[ It does not say intend to that.  It says to return `noindent` when
  indentation is impossible and gives "inside a string" as an example of
  a reason why indentation might not be possible.  ]

In any case, this argues in favor of `electric-indent-mode` calling
`indent-line-function` even when we're inside a comment.

The (nth 8 (syntax-ppss)) test has been in `electric-indent-mode` from
the very beginning, but I must say I can't remember why I put it in,
I think it was a mistake and we should remove it.

Especially since it can be re-added easily by configuring
`electric-indent-functions` for those modes/users who prefer it that way.


        Stefan

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  larsi@gnus.org,
>   robin.neatherway@gmail.com,  20846@debbugs.gnu.org
> Date: Fri, 28 May 2021 10:09:41 -0400
> 
> > Then that sentence is misleading and should be rephrased, because my
> > reading of it is that it clearly states that auto-indentation inside
> > strings is generally impossible, and expects the implementations to
> > behave like that.
> 
> [ It does not say intend to that.  It says to return `noindent` when
>   indentation is impossible and gives "inside a string" as an example of
>   a reason why indentation might not be possible.  ]

Then how about saying the below instead?

  If it is called somewhere where it cannot auto-indent, the function
  should return `noindent' to signal that it didn't.

(I intentionally removed the example because I think it isn't really
needed, and is actually harmful in this case.)

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The (nth 8 (syntax-ppss)) test has been in `electric-indent-mode` from
> the very beginning, but I must say I can't remember why I put it in,
> I think it was a mistake and we should remove it.

This bit?

    (when (and
[...]
               (not
                (or (memq act '(nil no-indent))
                    ;; In a string or comment.
                    (unless (eq act 'do-indent) (nth 8 (syntax-ppss))))))))


Uhm...  I stared at that logic for a couple of minutes, but I'm still
not able to convince myself that I understand it, so could you do the
necessary here?  :-)

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

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-05-29 04:26:29] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> The (nth 8 (syntax-ppss)) test has been in `electric-indent-mode` from
>> the very beginning, but I must say I can't remember why I put it in,
>> I think it was a mistake and we should remove it.
>
> This bit?
>
>     (when (and
> [...]
>                (not
>                 (or (memq act '(nil no-indent))
>                     ;; In a string or comment.
>                     (unless (eq act 'do-indent) (nth 8 (syntax-ppss))))))))
>
>
> Uhm...  I stared at that logic for a couple of minutes, but I'm still
> not able to convince myself that I understand it,

The logic here is that the functions on the hook can return either nil
to mean "indent, of course" or `no-indent` or `do-indent` where this
last one overrides the default "don't indent inside a string or comment"
(and it also overrides potential other functions on that hook which
might have said `no-indent`).

> so could you do the necessary here?  :-)

Done.  Can we close this bug?


        Stefan

bug#20846: 24.4; Electric-indent-mode does not call indent-line-function after hitting RET inside a comment

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The logic here is that the functions on the hook can return either nil
> to mean "indent, of course" or `no-indent` or `do-indent` where this
> last one overrides the default "don't indent inside a string or comment"
> (and it also overrides potential other functions on that hook which
> might have said `no-indent`).

Right.  By the way, `electric-indent-functions' talks about the
`no-indent' symbol, while `indent-line-function' talks about the
`noindent' symbol...  but skimming the code, I guess they're used in
different contexts, so that's not actually an issue.

>> so could you do the necessary here?  :-)
>
> Done.  Can we close this bug?

Yup; done now.

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