bug#64589: 30.0.50; smie-config-guess can break ruby indentation

bug#64589: 30.0.50; smie-config-guess can break ruby indentation

[Aaron Jensen]

In repro.rb:

def some_method
  return some_other_method(
    some_argument
  )
end

emacs -Q repro.rb
M-x smie-config-guess

Type:

foo(
x
)

The x will not be indented. If smie-config-guess is omitted, it indents
as expected.



In GNU Emacs 30.0.50 (build 55, aarch64-apple-darwin22.5.0, NS
 appkit-2299.60 Version 13.4.1 (Build 22F82)) of 2023-07-12 built on
 Aarons-Laptop.local
Repository revision: d861de72289464e336c772f1741016a90a2118c5
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.4.1

Configured using:
 'configure --with-ns --with-native-compilation --with-modules
 --without-dbus --without-webp --disable-ns-self-contained'

bug#64589: 30.0.50; smie-config-guess can break ruby indentation

[Dmitry Gutov]

Hi Aaron,

On 13/07/2023 07:20, Aaron Jensen wrote:
> In repro.rb:
> 
> def some_method
>    return some_other_method(
>      some_argument
>    )
> end
> 
> emacs -Q repro.rb
> M-x smie-config-guess
> 
> Type:
> 
> foo(
> x
> )
> 
> The x will not be indented. If smie-config-guess is omitted, it indents
> as expected.

Sorry about the late reply.

But it seems like this feature might be incompatible with the relatively 
complex way the ruby-mode indentation is written (the comments inside 
'smie-config--guess' seem to indicate so). So while it does sound cool 
to be able to use style detection, it might only work okay for certain 
languages/indentation rules.

One thing to note, though, is that it seems to want larger files, to be 
able to "learn" from more cases. But when I provide a larger file, it 
usually ends up saying "Nothing to change", even if some lines are 
indented differently from the current config. E.g. if I just expand your 
example to:

   def some_method
     return some_other_method(
       some_argument
     )

     foo(
       bar,
       tee
     )
   end

then adding foo(<RET> at the end indents predictably okay.

Perhaps Stefan as the author could tell us whether it's at all feasible 
to adjust ruby-mode to be more compatible with smie-config-guesser, or 
whether there could be some low-hanging fruit in the latter to help.

bug#64589: 30.0.50; smie-config-guess can break ruby indentation

[Aaron Jensen]

On Thu, Oct 19, 2023 at 4:05 PM Dmitry Gutov <dmitry@gutov.dev> wrote:
>
> Hi Aaron,
>
> On 13/07/2023 07:20, Aaron Jensen wrote:
> > In repro.rb:
> >
> > def some_method
> >    return some_other_method(
> >      some_argument
> >    )
> > end
> >
> > emacs -Q repro.rb
> > M-x smie-config-guess
> >
> > Type:
> >
> > foo(
> > x
> > )
> >
> > The x will not be indented. If smie-config-guess is omitted, it indents
> > as expected.
>
> Sorry about the late reply.
>
> But it seems like this feature might be incompatible with the relatively
> complex way the ruby-mode indentation is written (the comments inside
> 'smie-config--guess' seem to indicate so). So while it does sound cool
> to be able to use style detection, it might only work okay for certain
> languages/indentation rules.
>
> One thing to note, though, is that it seems to want larger files, to be
> able to "learn" from more cases. But when I provide a larger file, it
> usually ends up saying "Nothing to change", even if some lines are
> indented differently from the current config. E.g. if I just expand your
> example to:
>
>    def some_method
>      return some_other_method(
>        some_argument
>      )
>
>      foo(
>        bar,
>        tee
>      )
>    end
>
> then adding foo(<RET> at the end indents predictably okay.
>
> Perhaps Stefan as the author could tell us whether it's at all feasible
> to adjust ruby-mode to be more compatible with smie-config-guesser, or
> whether there could be some low-hanging fruit in the latter to help.

Ok, thanks. The context here is that I use dtrt-indent and it invokes
this before doing its work. For now, I have disabled it in Ruby (which
I think is actually fine because I thankfully don't work with any Ruby
code that is something other than 2 spaces).

So right now, I'm not concerned about this personally, but it does
seem odd that it breaks indentation in some cases.

Thanks,

Aaron

bug#64589: 30.0.50; smie-config-guess can break ruby indentation

[Dmitry Gutov]

On 20/10/2023 00:41, Aaron Jensen wrote:
> Ok, thanks. The context here is that I use dtrt-indent and it invokes
> this before doing its work. For now, I have disabled it in Ruby (which
> I think is actually fine because I thankfully don't work with any Ruby
> code that is something other than 2 spaces).

All right.

> So right now, I'm not concerned about this personally, but it does
> seem odd that it breaks indentation in some cases.

Perhaps we'll simply end up documenting that it doesn't work for all 
SMIE modes, just for some. And dtrt-indent will have a whitelist.

bug#64589: 30.0.50; smie-config-guess can break ruby indentation

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

> Perhaps we'll simply end up documenting that it doesn't work for all SMIE
> modes, just for some. And dtrt-indent will have a whitelist.

It's definitely the case that it's a "best effort" kind of tool, which
makes assumptions that don't always hold.
Using it unconditionally like `dtrt-indent` does seems ... optimistic,
but I'll take it as a compliment :-)


        Stefan

bug#64589: 30.0.50; smie-config-guess can break ruby indentation

[Aaron Jensen]

On Thu, Oct 19, 2023 at 10:15 PM Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
> > Perhaps we'll simply end up documenting that it doesn't work for all SMIE
> > modes, just for some. And dtrt-indent will have a whitelist.
>
> It's definitely the case that it's a "best effort" kind of tool, which
> makes assumptions that don't always hold.
> Using it unconditionally like `dtrt-indent` does seems ... optimistic,
> but I'll take it as a compliment :-)
>
>
>         Stefan
>

Just to double check, the thing I found odd was that it broke some
indentation, but not all. I don't know anything about how it works or
what its impact is, but it wasn't like it disabled indentation across
the board. It just made much of the indentation stop working or break
in subtle ways.

This is what smie-config-buffer-local ends up as: ((-5 :elem basic 2))

Thanks,

Aaron