bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Mohammed Sadiq]

The else block in the following code is not indented on TAB:

int
main (void)
{
   if (true)
     do_something ();
   else
do_something_else ();
}

How to reproduce:
1. Select the complete buffer
2. Press TAB

Expected result:
do_something_else() should be indented

afair, this did seem to work in the past.  May be this
happened after I updated treesitter-c module, idk.


In GNU Emacs 29.1.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
  3.24.37, cairo version 1.16.0) of 2023-08-01 built on purism
Repository revision: 0c29f53ab8723dd5a9f31ce8a6e913cc08132e56
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 
11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
  'configure --prefix=/usr'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

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

Major mode: C/*

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
   line-number-mode: t
   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 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 subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
c-ts-mode c-ts-common treesit cl-seq 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 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 lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 59978 9655)
  (symbols 48 7209 0)
  (strings 32 20385 1935)
  (string-bytes 1 740617)
  (vectors 16 12951)
  (vector-slots 8 184640 13476)
  (floats 8 24 23)
  (intervals 56 252 0)
  (buffers 984 11))

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Eli Zaretskii]

> Date: Wed, 02 Aug 2023 08:13:08 +0530
> From: Mohammed Sadiq <sadiq@sadiqpk.org>
> 
> The else block in the following code is not indented on TAB:
> 
> int
> main (void)
> {
>    if (true)
>      do_something ();
>    else
> do_something_else ();
> }
> 
> How to reproduce:
> 1. Select the complete buffer
> 2. Press TAB
> 
> Expected result:
> do_something_else() should be indented
> 
> afair, this did seem to work in the past.  May be this
> happened after I updated treesitter-c module, idk.

Thank you for your report.

Yuan, can you look into this, please?

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

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

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

Here is a patch that makes it work and fixes a couple of tests that were
failing in the BSD style. Hope it helps.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: c-ts-mode-indent-patch --]
[-- Type: text/x-patch, Size: 2162 bytes --]

From 43f63fa9fdb5c6e77fd8928a521686ea0b847b2f Mon Sep 17 00:00:00 2001
From: john muhl <jm@pub.pink>
Date: Wed, 2 Aug 2023 10:30:22 -0500
Subject: [PATCH] Fix indentation after else

* lisp/progmodes/c-ts-mode.el (c-ts-mode--indent-styles):
Indent statement after else_clause.
---
 lisp/progmodes/c-ts-mode.el                         |  2 ++
 test/lisp/progmodes/c-ts-mode-resources/indent.erts | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index f16e06942b9..95d3d43c096 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -452,6 +452,7 @@ c-ts-mode--indent-styles
            ;; These rules are for cases where the body is bracketless.
            ;; Tested by the "Bracketless Simple Statement" test.
            ((parent-is "if_statement") standalone-parent c-ts-mode-indent-offset)
+           ((parent-is "else_clause") standalone-parent c-ts-mode-indent-offset)
            ((parent-is "for_statement") standalone-parent c-ts-mode-indent-offset)
            ((parent-is "while_statement") standalone-parent c-ts-mode-indent-offset)
            ((parent-is "do_statement") standalone-parent c-ts-mode-indent-offset)
@@ -478,6 +479,7 @@ c-ts-mode--indent-styles
        ((parent-is "labeled_statement") parent-bol c-ts-mode-indent-offset)
        ((parent-is "compound_statement") parent-bol c-ts-mode-indent-offset)
        ((parent-is "if_statement") parent-bol 0)
+       ((parent-is "else_clause") parent-bol 0)
        ((parent-is "for_statement") parent-bol 0)
        ((parent-is "while_statement") parent-bol 0)
        ((parent-is "switch_statement") parent-bol 0)
diff --git a/test/lisp/progmodes/c-ts-mode-resources/indent.erts b/test/lisp/progmodes/c-ts-mode-resources/indent.erts
index 221b3d809af..b6a30b8f969 100644
--- a/test/lisp/progmodes/c-ts-mode-resources/indent.erts
+++ b/test/lisp/progmodes/c-ts-mode-resources/indent.erts
@@ -478,3 +478,16 @@ main (void)
 {
   |
 =-=-=
+
+Name: Bracketless If/Else Clause
+
+=-=
+int
+main (void)
+{
+  if (true)
+    do_something ();
+  else
+    do_something_else ();
+}
+=-=-=
-- 
2.41.0

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Yuan Fu]

> On Aug 2, 2023, at 4:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> Date: Wed, 02 Aug 2023 08:13:08 +0530
>> From: Mohammed Sadiq <sadiq@sadiqpk.org>
>> 
>> The else block in the following code is not indented on TAB:
>> 
>> int
>> main (void)
>> {
>>   if (true)
>>     do_something ();
>>   else
>> do_something_else ();
>> }
>> 
>> How to reproduce:
>> 1. Select the complete buffer
>> 2. Press TAB
>> 
>> Expected result:
>> do_something_else() should be indented
>> 
>> afair, this did seem to work in the past.  May be this
>> happened after I updated treesitter-c module, idk.

Yeah, (sign) I can reproduce this with the latest tree-sitter-c grammar but not the old one. Someone decides to add an else_clause node into the grammar [1] two weeks ago.

> 
> Thank you for your report.
> 
> Yuan, can you look into this, please?

Should the fix go into emacs-29 or master?

We really need some way to mandate a version of grammar. These breaking changes are far more frequent than I originally thought.

Yuan

[1] https://github.com/tree-sitter/tree-sitter-c/pull/115

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 2 Aug 2023 09:46:54 -0700
> Cc: Mohammed Sadiq <sadiq@sadiqpk.org>,
>  65006@debbugs.gnu.org
> 
> >> afair, this did seem to work in the past.  May be this
> >> happened after I updated treesitter-c module, idk.
> 
> Yeah, (sign) I can reproduce this with the latest tree-sitter-c grammar but not the old one. Someone decides to add an else_clause node into the grammar [1] two weeks ago.

How was the else clause parsed in the previous versions of the
grammar?

Will the proposed fix work with the older versions of the grammar?

> > Yuan, can you look into this, please?
> 
> Should the fix go into emacs-29 or master?

To emacs-29, please.

> We really need some way to mandate a version of grammar. These breaking changes are far more frequent than I originally thought.

Who will track all those versions and record which ones are supported?
And many grammar libraries don't have versions at all, so we will have
to track commits instead.

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Yuan Fu]

> On Aug 2, 2023, at 10:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Wed, 2 Aug 2023 09:46:54 -0700
>> Cc: Mohammed Sadiq <sadiq@sadiqpk.org>,
>> 65006@debbugs.gnu.org
>> 
>>>> afair, this did seem to work in the past.  May be this
>>>> happened after I updated treesitter-c module, idk.
>> 
>> Yeah, (sign) I can reproduce this with the latest tree-sitter-c grammar but not the old one. Someone decides to add an else_clause node into the grammar [1] two weeks ago.
> 
> How was the else clause parsed in the previous versions of the
> grammar?

It used to be something like this:

   (if_statement
     if
     condition: (xxx)
     consequence:  (xxx)
     else
     alternative: (xxx))

Now it’s like this:

   (if_statement
     if
     condition: (xxx)
     consequence:  (xxx)
     alternative: 
      (else_clause else
       (xxx)))

For the reason of the change, you can check out the link in my previous email. The justification is IMO insubstantial, to say the least :-(

> Will the proposed fix work with the older versions of the grammar?

Yes

>>> Yuan, can you look into this, please?
>> 
>> Should the fix go into emacs-29 or master?
> 
> To emacs-29, please.

Ok.

>> We really need some way to mandate a version of grammar. These breaking changes are far more frequent than I originally thought.
> 
> Who will track all those versions and record which ones are supported?
> And many grammar libraries don't have versions at all, so we will have
> to track commits instead.

We can request tree-sitter to add versioning to languages, which won’t happen any time soon; or we can pin the commits (which is what neovim does), but I know that’s against our policy. I don’t really have a good solution. But having our packages randomly break all the time is bad, and keep adapting all these breaking changes is also a significant burden on Emacs maintainers. And the speed in which we fix these breakages can never match the speed in which they bring them.

Yuan

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 2 Aug 2023 17:45:10 -0700
> Cc: Mohammed Sadiq <sadiq@sadiqpk.org>,
>  65006@debbugs.gnu.org
> 
> > On Aug 2, 2023, at 10:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > 
> >> From: Yuan Fu <casouri@gmail.com>
> >> Date: Wed, 2 Aug 2023 09:46:54 -0700
> >> Cc: Mohammed Sadiq <sadiq@sadiqpk.org>,
> >> 65006@debbugs.gnu.org
> >> 
> >>>> afair, this did seem to work in the past.  May be this
> >>>> happened after I updated treesitter-c module, idk.
> >> 
> >> Yeah, (sign) I can reproduce this with the latest tree-sitter-c grammar but not the old one. Someone decides to add an else_clause node into the grammar [1] two weeks ago.
> > 
> > How was the else clause parsed in the previous versions of the
> > grammar?
> 
> It used to be something like this:
> 
>    (if_statement
>      if
>      condition: (xxx)
>      consequence:  (xxx)
>      else
>      alternative: (xxx))
> 
> Now it’s like this:
> 
>    (if_statement
>      if
>      condition: (xxx)
>      consequence:  (xxx)
>      alternative: 
>       (else_clause else
>        (xxx)))
> 
> For the reason of the change, you can check out the link in my previous email. The justification is IMO insubstantial, to say the least :-(

Maybe we should track all those discussions, and voice our opinion
against changes whose reasons are not strong enough to justify
breakage?  For example, this particular PR was around since Sep 2022,
so we should have had ample time to voice our objections.

OTOH, they say that parsers for other languages already have
else_clause in their grammars.  So maybe this change is somewhat to
the better, at least when features common to several languages are
considered?

And anyway, this again raises the issue of Someone™ volunteering to
keep track of these developments.  We already have 14 TS-based modes
in Emacs (on master), so this is not a trivial job, I think.

> > Will the proposed fix work with the older versions of the grammar?
> 
> Yes
> 
> >>> Yuan, can you look into this, please?
> >> 
> >> Should the fix go into emacs-29 or master?
> > 
> > To emacs-29, please.
> 
> Ok.

Given that the change doesn't affect old parsers, please install on
the emacs-29 branch as soon as you are okay with the changes.

> >> We really need some way to mandate a version of grammar. These breaking changes are far more frequent than I originally thought.
> > 
> > Who will track all those versions and record which ones are supported?
> > And many grammar libraries don't have versions at all, so we will have
> > to track commits instead.
> 
> We can request tree-sitter to add versioning to languages, which won’t happen any time soon; or we can pin the commits (which is what neovim does), but I know that’s against our policy. I don’t really have a good solution. But having our packages randomly break all the time is bad, and keep adapting all these breaking changes is also a significant burden on Emacs maintainers. And the speed in which we fix these breakages can never match the speed in which they bring them.

I agree that we have a problem here.  The only question is how to
solve it in a reasonable way that will hold given our development and
release schedules, and given the schedules of users and distros
upgrading the Emacs versions.

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Yuan Fu]

> On Aug 2, 2023, at 9:03 AM, john muhl via Bug reports for GNU Emacs, the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
> 
> Here is a patch that makes it work and fixes a couple of tests that were
> failing in the BSD style. Hope it helps.
> 
> <0001-Fix-indentation-after-else.patch>

Thanks, that’s what I would do and it has a test! John, do you have FSF assignment/git write access?

Yuan

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

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

Yuan Fu <casouri@gmail.com> writes:

>> On Aug 2, 2023, at 9:03 AM, john muhl via Bug reports for GNU Emacs,
>> the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
>> 
>> Here is a patch that makes it work and fixes a couple of tests that were
>> failing in the BSD style. Hope it helps.
>> 
>> <0001-Fix-indentation-after-else.patch>
>
> Thanks, that’s what I would do and it has a test! John, do you have
> FSF assignment/git write access?

Yes on the assignment. No on git write access.

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

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

Eli Zaretskii <eliz@gnu.org> writes:

> Maybe we should track all those discussions, and voice our opinion
> against changes whose reasons are not strong enough to justify
> breakage?  For example, this particular PR was around since Sep 2022,
> so we should have had ample time to voice our objections.
>
> And anyway, this again raises the issue of Someone™ volunteering to
> keep track of these developments.  We already have 14 TS-based modes
> in Emacs (on master), so this is not a trivial job, I think.

I’m now subscribed to all of the relevant grammar repos and will try to
keep up with changes there. I’m probably not the right person to voice
an opinion on them without consulting higher authorities but hopefully
that part won’t be required very often.

> I agree that we have a problem here.  The only question is how to
> solve it in a reasonable way that will hold given our development and
> release schedules, and given the schedules of users and distros
> upgrading the Emacs versions.

Do you think the benefit of having ts modes be core packages would be
enough to justify whatever extra work that entails? It wouldn’t help
with syncing up release schedules but “install the latest version from
ELPA” might be enough better of an answer than “wait for the next Emacs
release or build from master” for most people.

bug#65006: 29.1.50; c-ts-mode: else block not indented right on TAB

[Eli Zaretskii]

> From: john muhl <jm@pub.pink>
> Cc: Yuan Fu <casouri@gmail.com>, 65006@debbugs.gnu.org
> Date: Mon, 07 Aug 2023 14:26:11 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Maybe we should track all those discussions, and voice our opinion
> > against changes whose reasons are not strong enough to justify
> > breakage?  For example, this particular PR was around since Sep 2022,
> > so we should have had ample time to voice our objections.
> >
> > And anyway, this again raises the issue of Someone™ volunteering to
> > keep track of these developments.  We already have 14 TS-based modes
> > in Emacs (on master), so this is not a trivial job, I think.
> 
> I’m now subscribed to all of the relevant grammar repos and will try to
> keep up with changes there. I’m probably not the right person to voice
> an opinion on them without consulting higher authorities but hopefully
> that part won’t be required very often.

Thank you.  Please post here whenever you see some changes planned
that might affect Emacs, and we will take it from there.

> > I agree that we have a problem here.  The only question is how to
> > solve it in a reasonable way that will hold given our development and
> > release schedules, and given the schedules of users and distros
> > upgrading the Emacs versions.
> 
> Do you think the benefit of having ts modes be core packages would be
> enough to justify whatever extra work that entails? It wouldn’t help
> with syncing up release schedules but “install the latest version from
> ELPA” might be enough better of an answer than “wait for the next Emacs
> release or build from master” for most people.

I think it's too early for us to make such conclusions.  We need to
collect more data points and experience.