bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Jostein Kjønigsen]

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


When using typescript-ts-mode in a bare config without any custom hooks defined,
RET ends up being bound (by default) to `newline`.

This is suboptimal, given that users typically want newlines to be indented.

RET should instead be bound to `newline-and-indent`.



In GNU Emacs 31.0.50 (build 4, aarch64-apple-darwin23.4.0, NS
 appkit-2487.50 Version 14.4.1 (Build 23E224)) of 2024-07-18 built on
 SOK67R3KWV97
Repository revision: 14bfa9edeb2ba47d0fb930cc708c5e4033100a54
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2487
System Description:  macOS 14.4.1

Configured using:
 'configure --with-json --with-tree-sitter --with-native-compilation
 PKG_CONFIG_PATH=:/usr/local/lib/pkgconfig:/opt/local/lib/pkgconfig'

Configured features:
ACL GLIB GNUTLS LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY KQUEUE NS
PDUMPER PNG RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS TREE_SITTER WEBP
XIM ZLIB

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

Major mode: ELisp/l

Minor modes in effect:
  bug-reference-prog-mode: t
  elisp-slime-nav-mode: t
  highlight-symbol-mode: t
  flycheck-mode: t
  company-mode: t
  which-function-mode: t
  copilot-mode: t
  editorconfig-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  async-bytecomp-package-mode: t
  delete-selection-mode: t
  global-auto-revert-mode: t
  paredit-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-nlinum-mode: t
  nlinum-mode: t
  ido-yes-or-no-mode: t
  override-global-mode: t
  server-mode: t
  global-hl-line-mode: t
  pixel-scroll-precision-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  hs-minor-mode: t

Load-path shadows:
/Users/josteink/.emacs.d/elpa/transient-20240421.1327/transient hides /Users/josteink/build/emacs/lisp/transient
/Users/josteink/.emacs.d/elpa/editorconfig-20240318.2049/editorconfig hides /Users/josteink/build/emacs/lisp/editorconfig
/Users/josteink/.emacs.d/elpa/editorconfig-20240318.2049/editorconfig-conf-mode hides /Users/josteink/build/emacs/lisp/editorconfig-conf-mode
/Users/josteink/.emacs.d/elpa/editorconfig-20240318.2049/editorconfig-core hides /Users/josteink/build/emacs/lisp/editorconfig-core
/Users/josteink/.emacs.d/elpa/editorconfig-20240318.2049/editorconfig-core-handle hides /Users/josteink/build/emacs/lisp/editorconfig-core-handle
/Users/josteink/.emacs.d/elpa/editorconfig-20240318.2049/editorconfig-fnmatch hides /Users/josteink/build/emacs/lisp/editorconfig-fnmatch

Features:
(shadow sort mail-extr emacsbug em-unix em-term term ehelp em-script
em-prompt em-pred em-ls em-hist em-glob em-extpipe em-cmpl em-dirs
em-basic em-banner em-alias esh-mode esh-var eshell esh-cmd esh-ext
esh-proc esh-opt esh-io esh-arg esh-module esh-module-loaddefs esh-util
helm-command helm-elisp helm-eval edebug helm-info dired-aux image-file
image-converter helm-external helm-net ffap bug-reference two-column
misearch multi-isearch help-fns radix-tree markdown-mode color add-log
combobulate combobulate-yaml combobulate-css combobulate-js-ts
combobulate-python combobulate-html combobulate-ui transient
combobulate-display let-alist combobulate-contrib multiple-cursors
mc-separate-operations rectangular-region-mode mc-mark-pop mc-edit-lines
mc-hide-unmatched-lines-mode mc-mark-more sgml-mode facemenu
mc-cycle-cursors multiple-cursors-core rect combobulate-manipulation
python combobulate-navigation combobulate-misc combobulate-interface
combobulate-rules combobulate-settings tempo elec-pair
typescript-ts-mode js c-ts-common cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs vc-git
vc-dispatcher winner tramp-archive tramp-gvfs ido-completing-read+
memoize cus-edit cus-start cus-load minibuf-eldef org-duration diary-lib
diary-loaddefs cal-iso disp-table oc-basic ol-eww ol-rmail ol-mhe ol-irc
ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime
gnutls dig gnus-sum gnus-group gnus-undo gnus-start gnus-dbus dbus
gnus-cloud nnimap nnmail mail-source utf7 nnoo gnus-spec gnus-int
gnus-range message sendmail yank-media rfc822 mml mml-sec epa derived
epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231
rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win
ol-docview doc-view jka-compr ol-bibtex bibtex ol-bbdb ol-w3m ol-doi
org-link-doi face-remap org-agenda org-element org-persist org-id
org-element-ast inline avl-tree org-refile elisp-slime-nav
highlight-symbol flycheck company-oddmuse company-keywords company-etags
etags fileloop generator company-gtags company-dabbrev-code
company-dabbrev company-files company-clang company-capf company-cmake
company-semantic company-template company-bbdb company eglot
external-completion flymake compat diff diff-mode track-changes ert pp
ewoc debug backtrace which-func hideshow copilot copilot-balancer
editorconfig editorconfig-core editorconfig-core-handle
editorconfig-fnmatch f dash s jsonrpc eww url-queue shr pixel-fill
kinsoku url-file svg xml dom puny mm-url gnus nnheader gnus-util
mail-utils range wid-edit mm-util mail-prsvr helm-imenu helm-mode
helm-misc helm-files image-dired image-dired-tags image-dired-external
image-dired-util image-mode dired dired-loaddefs exif tramp trampver
tramp-integration files-x tramp-message tramp-compat xdg shell
parse-time iso8601 tramp-loaddefs helm-buffers helm-occur helm-tags
helm-locate helm-grep helm-regexp helm-utils helm-help helm-types helm
helm-global-bindings helm-easymenu helm-core async-bytecomp helm-source
helm-multi-match helm-lib async imenu ob-plantuml delsel autorevert
filenotify org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-src ob-comint org-pcomplete pcomplete org-list org-footnote
org-faces org-entities noutline outline org-version ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys
oc org-loaddefs thingatpt find-func cal-menu calendar cal-loaddefs
org-compat org-macs format-spec yasnippet paredit dockerfile-mode
sh-script smie treesit executable nlinum linum ido-yes-or-no advice ido
edmacro kmacro use-package-bind-key bind-key easy-mmode xref project
server hl-line pixel-scroll cua-base compile-eslint all-the-icons
all-the-icons-faces data-material data-weathericons data-octicons
data-fileicons data-faicons data-alltheicons dracula-theme
use-package-ensure use-package-core finder-inf all-the-icons-autoloads
bmx-mode-autoloads cargo-autoloads cmake-mode-autoloads
company-autoloads crontab-mode-autoloads dap-mode-autoloads
bui-autoloads dockerfile-mode-autoloads doom-modeline-autoloads
dracula-theme-autoloads editorconfig-autoloads elisp-slime-nav-autoloads
expand-region-autoloads flycheck-autoloads helm-autoloads
helm-core-autoloads async-autoloads highlight-symbol-autoloads
ido-yes-or-no-autoloads ido-completing-read+-autoloads compile
text-property-search comint ansi-osc ansi-color ring time-date comp
comp-cstr cl-extra help-mode warnings comp-run comp-common rx
lsp-docker-autoloads lsp-treemacs-autoloads lsp-mode-autoloads
magit-autoloads pcase git-commit-autoloads magit-section-autoloads
markdown-mode-autoloads memoize-autoloads multiple-cursors-autoloads
nerd-icons-autoloads nlinum-autoloads paredit-autoloads
pkg-info-autoloads epl-autoloads popup-autoloads powershell-autoloads
shrink-path-autoloads f-autoloads spinner-autoloads swift-mode-autoloads
transient-autoloads treemacs-autoloads cfrs-autoloads posframe-autoloads
ht-autoloads hydra-autoloads lv-autoloads pfuture-autoloads
ace-window-autoloads avy-autoloads s-autoloads dash-autoloads
undo-tree-autoloads queue-autoloads wfnames-autoloads info
with-editor-autoloads yaml-autoloads yaml-mode-autoloads
yasnippet-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs icons password-cache json subr-x map byte-opt
gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/ns-win ns-win ucs-normalize
mule-util term/common-win 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 kqueue cocoa ns lcms2 multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 1458191 193746) (symbols 48 66974 1)
 (strings 32 418444 16703) (string-bytes 1 11887433)
 (vectors 16 160853) (vector-slots 8 2461125 186875)
 (floats 8 2169 354) (intervals 56 27943 2331) (buffers 992 53))

—
Kind Regards
Jostein Kjønigsen


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

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Andrea Corallo]

Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:

> When using typescript-ts-mode in a bare config without any custom hooks defined,
> RET ends up being bound (by default) to `newline`.
>
> This is suboptimal, given that users typically want newlines to be indented.
>
> RET should instead be bound to `newline-and-indent`.

Hi,

AFAIK this is what we tipically do, like: c-mode, c-ts-mode, lisp-mode,
emacs-lisp-mode etc.  Why 'typescript-ts-mode' should be different?

  Andrea

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Dmitry Gutov]

On 19/07/2024 00:30, Andrea Corallo wrote:
> Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
> 
>> When using typescript-ts-mode in a bare config without any custom hooks defined,
>> RET ends up being bound (by default) to `newline`.
>>
>> This is suboptimal, given that users typically want newlines to be indented.
>>
>> RET should instead be bound to `newline-and-indent`.
> Hi,
> 
> AFAIK this is what we tipically do, like: c-mode, c-ts-mode, lisp-mode,
> emacs-lisp-mode etc.  Why 'typescript-ts-mode' should be different?

Right, we normally handle automatic indentation using 
electric-indent-mode (which is on by default).

If RET behaves incorrectly in typescript-ts-mode somehow, more details 
should be provided.

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Eli Zaretskii]

> Cc: 72184@debbugs.gnu.org
> Date: Sat, 20 Jul 2024 03:22:47 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 19/07/2024 00:30, Andrea Corallo wrote:
> > Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
> > 
> >> When using typescript-ts-mode in a bare config without any custom hooks defined,
> >> RET ends up being bound (by default) to `newline`.
> >>
> >> This is suboptimal, given that users typically want newlines to be indented.
> >>
> >> RET should instead be bound to `newline-and-indent`.
> > Hi,
> > 
> > AFAIK this is what we tipically do, like: c-mode, c-ts-mode, lisp-mode,
> > emacs-lisp-mode etc.  Why 'typescript-ts-mode' should be different?
> 
> Right, we normally handle automatic indentation using 
> electric-indent-mode (which is on by default).
> 
> If RET behaves incorrectly in typescript-ts-mode somehow, more details 
> should be provided.

Jostein, could you please respond, so that we could make progress
here?

Thanks.

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Jostein Kjønigsen]

I’m on a vacation with the family, and without a laptop. I won’t be able to provide more thorough feedback until I’m back and can re-test various configurations.

I might be able to look into this in a week or so. Is that good?


--
Jostein Kjønigsen
https://jostein.kjønigsen.net

> On 27 Jul 2024, at 09:21, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> 
>> 
>> Cc: 72184@debbugs.gnu.org
>> Date: Sat, 20 Jul 2024 03:22:47 +0300
>> From: Dmitry Gutov <dmitry@gutov.dev>
>> 
>>> On 19/07/2024 00:30, Andrea Corallo wrote:
>>> Jostein Kjønigsen<jostein@secure.kjonigsen.net>  writes:
>>> 
>>>> When using typescript-ts-mode in a bare config without any custom hooks defined,
>>>> RET ends up being bound (by default) to `newline`.
>>>> 
>>>> This is suboptimal, given that users typically want newlines to be indented.
>>>> 
>>>> RET should instead be bound to `newline-and-indent`.
>>> Hi,
>>> 
>>> AFAIK this is what we tipically do, like: c-mode, c-ts-mode, lisp-mode,
>>> emacs-lisp-mode etc.  Why 'typescript-ts-mode' should be different?
>> 
>> Right, we normally handle automatic indentation using
>> electric-indent-mode (which is on by default).
>> 
>> If RET behaves incorrectly in typescript-ts-mode somehow, more details
>> should be provided.
> 
> Jostein, could you please respond, so that we could make progress
> here?
> 
> Thanks.

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Eli Zaretskii]

> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> Date: Sat, 27 Jul 2024 13:50:19 +0200
> Cc: Dmitry Gutov <dmitry@gutov.dev>, acorallo@gnu.org, 72184@debbugs.gnu.org
> 
> I’m on a vacation with the family, and without a laptop. I won’t be able to provide more thorough feedback until I’m back and can re-test various configurations.
> 
> I might be able to look into this in a week or so. Is that good?

Sure, there's no rush.

Thanks.

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Jostein Kjønigsen]

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



> On 27 Jul 2024, at 09:21, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> Cc: 72184@debbugs.gnu.org
>> Date: Sat, 20 Jul 2024 03:22:47 +0300
>> From: Dmitry Gutov <dmitry@gutov.dev>
>> 
>> Right, we normally handle automatic indentation using 
>> electric-indent-mode (which is on by default).
>> 
>> If RET behaves incorrectly in typescript-ts-mode somehow, more details 
>> should be provided.
> 
> Jostein, could you please respond, so that we could make progress
> here?
> 
> Thanks.


PREFACE

I've now looked further into this issue, and what we're actually seeing here is not indentation code "not working", but indentation rules not taking into account incomplete parse-trees while code is being written.

So as the original bug-submitter, I will have to just declare that the original bug-description is clearly inaccurate. Sorry about that!

Below is my attempt to better express what is actually failing and why.

My hypothesis is that this could be an issue for other tree-sitter based major-modes as well, and maintainers of those modes are encouraged to check for similar issues.

If more major-modes than typescript-ts-mode is affected, the optimum solution may best be decided together, rather than individually per mode.

AS FOR THE BUG - DETAILS

Consider the following pre-existing typescript/javascript function:

function stuff() {
  // TODO implement
}

Using the typescript tree-sitter grammer, it gets the following nodes in its tree-sitter AST:

 (function_declaration function name: (identifier)
  parameters: (formal_parameters ( ))
  body: (statement_block { (comment) }))
 
This matches the rules for indentation in typescript-ts-mode and indents correctly regardless.

But in a bare config with only electric-indent-mode enabled (and not electric-pair-mode), trying to enter a new function will produce a incomplete syntax tree, due to the missing final brace.

The code will look like this:

function gnu2() {  // NOTE: no closing brace!


And the tree-sitter AST will then look like this until completed:

 (expression_statement
  (function function name: (identifier)
   parameters: (formal_parameters ( ))
   body: (statement_block { (comment) }))))

And typescript-ts-mode has no indentation rules for this AST. Thus pressing enter wont cause indentation, despite electric-indent-mode being enabled.

I suspect this sort of issue could possibly affect other treesitter based modes which has been developed mostly in existing configs with "all the bells and whistles" (like electric-pair-mode) enabled.

I'll hold off any fixes or suggestions for such until we have some reports from other maintainers, if that is ok?

--
Jostein

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

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Dmitry Gutov]

Hi Jostein,

On 31/07/2024 11:55, Jostein Kjønigsen wrote:

> PREFACE
> 
> I've now looked further into this issue, and what we're actually seeing 
> here is not indentation code "not working", but indentation rules not 
> taking into account incomplete parse-trees while code is being written.

Makes sense, thanks.

> So as the original bug-submitter, I will have to just declare that the 
> original bug-description is clearly inaccurate. Sorry about that!
> 
> Below is my attempt to better express what is actually failing and why.
> 
> *My hypothesis is that this could be an issue for other tree-sitter 
> based major-modes as well, and maintainers of those modes are encouraged 
> to check for similar issues.*
> 
> If more major-modes than typescript-ts-mode is affected, the optimum 
> solution may best be decided together, rather than individually per mode.
> 
> AS FOR THE BUG - DETAILS
> 
> Consider the following pre-existing typescript/javascript function:
> 
>     function stuff() {
>        // TODO implement
>     }
> 
> 
> Using the typescript tree-sitter grammer, it gets the following nodes in 
> its tree-sitter AST:
> 
>       (function_declaration function name: (identifier)
>        parameters: (formal_parameters ( ))
>        body: (statement_block { (comment) }))
> 
> This matches the rules for indentation in typescript-ts-mode and indents 
> correctly regardless.
> 
> But in a bare config with only electric-indent-mode enabled (and not 
> electric-pair-mode), trying to enter a new function will produce a 
> incomplete syntax tree, due to the missing final brace.
> 
> The code will look like this:
> 
>     function gnu2() {  // NOTE: no closing brace!
> 
> 
> And the tree-sitter AST will then look like this until completed:
> 
>       (expression_statement
>        (function function name: (identifier)
>         parameters: (formal_parameters ( ))
>         body: (statement_block { (comment) }))))
> 
> 
> And typescript-ts-mode has no indentation rules for this AST. *Thus 
> pressing enter wont cause indentation, despite electric-indent-mode 
> being enabled.*
> 
> I suspect this sort of issue could possibly affect other treesitter 
> based modes which has been developed mostly in existing configs with 
> "all the bells and whistles" (like electric-pair-mode) enabled.
> 
> I'll hold off any fixes or suggestions for such until we have some 
> reports from other maintainers, if that is ok?

We've seen this before - take a look at https://debbugs.gnu.org/62717, 
for example.

In short, some ad-hoc counter-measures can and sometimes are added 
(proposals welcome), but ultimately when we know we can't reliably 
distinguish problem situations, or can only detect a minor fraction of 
them, we're also okay giving up and recommending people use 
electric-pair-mode for better experience.

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Jostein Kjønigsen]

> On 1 Aug 2024, at 02:42, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> Hi Jostein,
> 
> 
> We've seen this before - take a look at https://debbugs.gnu.org/62717, for example.
> 
> In short, some ad-hoc counter-measures can and sometimes are added (proposals welcome), but ultimately when we know we can't reliably distinguish problem situations, or can only detect a minor fraction of them, we're also okay giving up and recommending people use electric-pair-mode for better experience.

Right. In that case I think I'm fine with just closing this one too.

It's not causing issues for me on a day-to-day basis, only on a completely clean setup, and I honestly don't think too many programmers are using Emacs that way.

--
Jostein

bug#72184: 31.0.50; typescript-ts-mode does not bind RET to newline-and-indent

[Eli Zaretskii]

> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> Date: Thu, 15 Aug 2024 12:03:25 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  acorallo@gnu.org,
>  72184@debbugs.gnu.org,
>  Yuan Fu <casouri@gmail.com>,
>  Theodor Thornhill <theo@thornhill.no>
> 
> 
> > On 1 Aug 2024, at 02:42, Dmitry Gutov <dmitry@gutov.dev> wrote:
> > 
> > Hi Jostein,
> > 
> > 
> > We've seen this before - take a look at https://debbugs.gnu.org/62717, for example.
> > 
> > In short, some ad-hoc counter-measures can and sometimes are added (proposals welcome), but ultimately when we know we can't reliably distinguish problem situations, or can only detect a minor fraction of them, we're also okay giving up and recommending people use electric-pair-mode for better experience.
> 
> Right. In that case I think I'm fine with just closing this one too.
> 
> It's not causing issues for me on a day-to-day basis, only on a completely clean setup, and I honestly don't think too many programmers are using Emacs that way.

OK, closing.