bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Michael Kleehammer]

In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
wrapping comments.

1) emacs -Q 

2) In the scratch buffer:
   a) M-x js-ts-mode
   b) M-x auto-fill-mode
   c) M-x erase-buffer
   
3) Paste in a long JS comment:

   // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it

4) At end of line, press <Space>.

The text wraps like so:

   // This buffer is for text that is not saved, and for Lisp evaluation.
   //   To create a file, visit it 

In previous versions, it would wrap like so:

   // This buffer is for text that is not saved, and for Lisp evaluation.
   // To create a file, visit it 


Thanks,
Michael Kleehammer

In GNU Emacs 30.0.91 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.33, cairo version 1.16.0) of 2024-10-14 built on pangolin
Repository revision: b87fda63dd4a29c3c28e235904405f2d6709239e
Repository branch: emacs-30
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Pop!_OS 22.04 LTS

Configured using:
 'configure --with-tree-sitter --without-compress-install
 --without-mailutils 'CFLAGS=-O2 -march=native''

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PNG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER
X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

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

Major mode: JavaScript

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-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
  minibuffer-regexp-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
comp-common rx emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-seq js
c-ts-common treesit json subr-x map byte-opt gv bytecomp byte-compile
imenu cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs cl-loaddefs cl-lib rmc iso-transl tooltip
cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd touch-screen
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine 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 composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
gtk x-toolkit xinput2 x multi-tty move-toolbar make-network-process
native-compile emacs)

Memory information:
((conses 16 112390 11199) (symbols 48 9369 0) (strings 32 27327 1317)
 (string-bytes 1 999282) (vectors 16 15439)
 (vector-slots 8 200959 7612) (floats 8 28 7) (intervals 56 277 0)
 (buffers 992 11))

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Eli Zaretskii]

> From: Michael Kleehammer <michael@kleehammer.com>
> Date: Sat, 19 Oct 2024 22:46:55 -0500
> 
> In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
> wrapping comments.
> 
> 1) emacs -Q 
> 
> 2) In the scratch buffer:
>    a) M-x js-ts-mode
>    b) M-x auto-fill-mode
>    c) M-x erase-buffer
>    
> 3) Paste in a long JS comment:
> 
>    // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it
> 
> 4) At end of line, press <Space>.
> 
> The text wraps like so:
> 
>    // This buffer is for text that is not saved, and for Lisp evaluation.
>    //   To create a file, visit it 
> 
> In previous versions, it would wrap like so:
> 
>    // This buffer is for text that is not saved, and for Lisp evaluation.
>    // To create a file, visit it 

Adding people who know about js-ts-mode, in the hope they will have
comments or suggestions.

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Yuan Fu]

> On Oct 27, 2024, at 3:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Michael Kleehammer <michael@kleehammer.com>
>> Date: Sat, 19 Oct 2024 22:46:55 -0500
>> 
>> In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
>> wrapping comments.
>> 
>> 1) emacs -Q 
>> 
>> 2) In the scratch buffer:
>>   a) M-x js-ts-mode
>>   b) M-x auto-fill-mode
>>   c) M-x erase-buffer
>> 
>> 3) Paste in a long JS comment:
>> 
>>   // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it
>> 
>> 4) At end of line, press <Space>.
>> 
>> The text wraps like so:
>> 
>>   // This buffer is for text that is not saved, and for Lisp evaluation.
>>   //   To create a file, visit it 
>> 
>> In previous versions, it would wrap like so:
>> 
>>   // This buffer is for text that is not saved, and for Lisp evaluation.
>>   // To create a file, visit it 
> 
> Adding people who know about js-ts-mode, in the hope they will have
> comments or suggestions.

Thanks Eli, I’ll look into it.

Yuan

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Dmitry Gutov]

On 27/10/2024 12:55, Eli Zaretskii wrote:
>> From: Michael Kleehammer<michael@kleehammer.com>
>> Date: Sat, 19 Oct 2024 22:46:55 -0500
>>
>> In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
>> wrapping comments.
>>
>> 1) emacs -Q
>>
>> 2) In the scratch buffer:
>>     a) M-x js-ts-mode
>>     b) M-x auto-fill-mode
>>     c) M-x erase-buffer
>>     
>> 3) Paste in a long JS comment:
>>
>>     // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it
>>
>> 4) At end of line, press <Space>.
>>
>> The text wraps like so:
>>
>>     // This buffer is for text that is not saved, and for Lisp evaluation.
>>     //   To create a file, visit it
>>
>> In previous versions, it would wrap like so:
>>
>>     // This buffer is for text that is not saved, and for Lisp evaluation.
>>     // To create a file, visit it
> Adding people who know about js-ts-mode, in the hope they will have
> comments or suggestions.

I suppose the answer is somewhere around the fact that we have both 
normal-auto-fill-function and fill-paragraph-function, and that 
js-ts-mode (through c-ts-common-comment-setup) only configures the 
latter, while js-mode (in js--mode-setup) has settings for both.

Not sure why the default do-auto-fill doesn't skip over the space, though.

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Yuan Fu]

> On Oct 27, 2024, at 8:10 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> On 27/10/2024 12:55, Eli Zaretskii wrote:
>>> From: Michael Kleehammer<michael@kleehammer.com>
>>> Date: Sat, 19 Oct 2024 22:46:55 -0500
>>> 
>>> In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
>>> wrapping comments.
>>> 
>>> 1) emacs -Q
>>> 
>>> 2) In the scratch buffer:
>>>    a) M-x js-ts-mode
>>>    b) M-x auto-fill-mode
>>>    c) M-x erase-buffer
>>>    3) Paste in a long JS comment:
>>> 
>>>    // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it
>>> 
>>> 4) At end of line, press <Space>.
>>> 
>>> The text wraps like so:
>>> 
>>>    // This buffer is for text that is not saved, and for Lisp evaluation.
>>>    //   To create a file, visit it
>>> 
>>> In previous versions, it would wrap like so:
>>> 
>>>    // This buffer is for text that is not saved, and for Lisp evaluation.
>>>    // To create a file, visit it
>> Adding people who know about js-ts-mode, in the hope they will have
>> comments or suggestions.
> 
> I suppose the answer is somewhere around the fact that we have both normal-auto-fill-function and fill-paragraph-function, and that js-ts-mode (through c-ts-common-comment-setup) only configures the latter, while js-mode (in js--mode-setup) has settings for both.
> 
> Not sure why the default do-auto-fill doesn't skip over the space, though.

Ok, so the actually difference is in comment-line-break-function, which auto-fill-mode uses to insert line break. In js-mode, it’s set to c-indent-new-comment-line, which deletes the whitespace; in tree-sitter mode, it’s c-ts-common-comment-indent-new-line, which doesn’t. I fixed c-ts-common-comment-indent-new-line to do the same thing as c-indent-new-comment-line.

However, I did the foolish thing of pushing to master. Should I cherry pick the commit into emacs-30? Would that create any merge conflict down the line?

Yuan

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Tue, 29 Oct 2024 00:33:05 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Michael Kleehammer <michael@kleehammer.com>,
>  73900@debbugs.gnu.org
> 
> 
> 
> > On Oct 27, 2024, at 8:10 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> > 
> > On 27/10/2024 12:55, Eli Zaretskii wrote:
> >>> From: Michael Kleehammer<michael@kleehammer.com>
> >>> Date: Sat, 19 Oct 2024 22:46:55 -0500
> >>> 
> >>> In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
> >>> wrapping comments.
> >>> 
> >>> 1) emacs -Q
> >>> 
> >>> 2) In the scratch buffer:
> >>>    a) M-x js-ts-mode
> >>>    b) M-x auto-fill-mode
> >>>    c) M-x erase-buffer
> >>>    3) Paste in a long JS comment:
> >>> 
> >>>    // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it
> >>> 
> >>> 4) At end of line, press <Space>.
> >>> 
> >>> The text wraps like so:
> >>> 
> >>>    // This buffer is for text that is not saved, and for Lisp evaluation.
> >>>    //   To create a file, visit it
> >>> 
> >>> In previous versions, it would wrap like so:
> >>> 
> >>>    // This buffer is for text that is not saved, and for Lisp evaluation.
> >>>    // To create a file, visit it
> >> Adding people who know about js-ts-mode, in the hope they will have
> >> comments or suggestions.
> > 
> > I suppose the answer is somewhere around the fact that we have both normal-auto-fill-function and fill-paragraph-function, and that js-ts-mode (through c-ts-common-comment-setup) only configures the latter, while js-mode (in js--mode-setup) has settings for both.
> > 
> > Not sure why the default do-auto-fill doesn't skip over the space, though.
> 
> Ok, so the actually difference is in comment-line-break-function, which auto-fill-mode uses to insert line break. In js-mode, it’s set to c-indent-new-comment-line, which deletes the whitespace; in tree-sitter mode, it’s c-ts-common-comment-indent-new-line, which doesn’t. I fixed c-ts-common-comment-indent-new-line to do the same thing as c-indent-new-comment-line.
> 
> However, I did the foolish thing of pushing to master. Should I cherry pick the commit into emacs-30?

Yes, please.  Just first fix the problem I reported a few minutes ago.

> Would that create any merge conflict down the line?

No.

bug#73900: 30.0.91; auto-fill adds extra space in js-ts-mode

[Yuan Fu]

> On Oct 29, 2024, at 5:39 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Tue, 29 Oct 2024 00:33:05 -0700
>> Cc: Eli Zaretskii <eliz@gnu.org>,
>> Michael Kleehammer <michael@kleehammer.com>,
>> 73900@debbugs.gnu.org
>> 
>> 
>> 
>>> On Oct 27, 2024, at 8:10 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
>>> 
>>> On 27/10/2024 12:55, Eli Zaretskii wrote:
>>>>> From: Michael Kleehammer<michael@kleehammer.com>
>>>>> Date: Sat, 19 Oct 2024 22:46:55 -0500
>>>>> 
>>>>> In Emacs 30, js-ts-mode auto-fill chooses an incorrect prefix when
>>>>> wrapping comments.
>>>>> 
>>>>> 1) emacs -Q
>>>>> 
>>>>> 2) In the scratch buffer:
>>>>>   a) M-x js-ts-mode
>>>>>   b) M-x auto-fill-mode
>>>>>   c) M-x erase-buffer
>>>>>   3) Paste in a long JS comment:
>>>>> 
>>>>>   // This buffer is for text that is not saved, and for Lisp evaluation.  To create a file, visit it
>>>>> 
>>>>> 4) At end of line, press <Space>.
>>>>> 
>>>>> The text wraps like so:
>>>>> 
>>>>>   // This buffer is for text that is not saved, and for Lisp evaluation.
>>>>>   //   To create a file, visit it
>>>>> 
>>>>> In previous versions, it would wrap like so:
>>>>> 
>>>>>   // This buffer is for text that is not saved, and for Lisp evaluation.
>>>>>   // To create a file, visit it
>>>> Adding people who know about js-ts-mode, in the hope they will have
>>>> comments or suggestions.
>>> 
>>> I suppose the answer is somewhere around the fact that we have both normal-auto-fill-function and fill-paragraph-function, and that js-ts-mode (through c-ts-common-comment-setup) only configures the latter, while js-mode (in js--mode-setup) has settings for both.
>>> 
>>> Not sure why the default do-auto-fill doesn't skip over the space, though.
>> 
>> Ok, so the actually difference is in comment-line-break-function, which auto-fill-mode uses to insert line break. In js-mode, it’s set to c-indent-new-comment-line, which deletes the whitespace; in tree-sitter mode, it’s c-ts-common-comment-indent-new-line, which doesn’t. I fixed c-ts-common-comment-indent-new-line to do the same thing as c-indent-new-comment-line.
>> 
>> However, I did the foolish thing of pushing to master. Should I cherry pick the commit into emacs-30?
> 
> Yes, please.  Just first fix the problem I reported a few minutes ago.
> 
>> Would that create any merge conflict down the line?
> 
> No.

Ok, thanks. I fixed that problem and pushed both to emacs-30.

Yuan