indentation problem in nested functions with line continuations

[Sec-ant]

MISS_HIT Component affected Please choose one from:

• Style checker

Your MATLAB/Octave environment

• MATLAB
• R2021a

Your operating system and Python version

• Windows
• Python 3.9.1

Describe the bug When I use mh_style with --fix option to style the following code

a = b(...
    c(...
    d(...
    e(...
    f(...
    g(...
    h...
    )...
    )...
    )...
    )...
    )...
    );

the output will be

a = b( ...
      c( ...
      d( ...
      e( ...
      f( ...
      g( ...
      h ...
     ) ...
     ) ...
     ) ...
     ) ...
     ) ...
     );

which is not a final steady indentation style and can be further styled multiple times till

a = b( ...
      c( ...
        d( ...
          e( ...
            f( ...
              g( ...
                h ...
               ) ...
             ) ...
           ) ...
         ) ...
       ) ...
     );

I'm assuming this is not intended and there may be some problem when calculating the indentation in this kind of nested functions with line continuations?

[Sec-ant]

And this kind of line continuations will just work fine (however the github matlab code syntax highlighting seems to be broken here):

before:

a = b...
    (c...
    (d...
    (e...
    (f...
    (g...
    (h...
    )...
    )...
    )...
    )...
    )...
    );

after:

a = b ...
    (c ...
     (d ...
      (e ...
       (f ...
        (g ...
         (h ...
         ) ...
        ) ...
       ) ...
      ) ...
     ) ...
    );

[florianschanda]

Yes, this is a known issue (see https://github.com/florianschanda/miss_hit/blob/master/CHANGELOG.md#tooling)

The way the indenter and pretty printer works is weird, and that largely explains this behaviour. It generally works by just adding or removing a few spaces; plus line continuations are really awkward in MATLAB anyway, and my handling of them is not ideal either.

I do plan to produce a totally new (ast-based) pretty printer; I just need to work out how to correctly deal with comments. But I can look at this case and see if I can maybe fix something, but please don't expect too much in the short term...

[Sec-ant]

Got it, no hurry. It's already an excellent work and fits my needs. Thanks!

[florianschanda]

I have a guess what is happening; the line continuation will be based on where the opening brace is; but we use the un-fixed position for determining the column number not the fixed one.

[florianschanda]

Ugh, as expected it's nasty. So the continuation is of course based on the last encountered bracket and we derive the indentation based on it's (fixed) location. But this logic only works if the bracket is the first thing in a line.

This will be very nasty to fix, but I will see what I can do.

[Sec-ant]

Ugh, as expected it's nasty. So the continuation is of course based on the last encountered bracket and we derive the indentation based on it's (fixed) location. But this logic only works if the bracket is the first thing in a line.

This will be very nasty to fix, but I will see what I can do.

👍 Good to know the reason!

And I find another case where similar problem occurs, I'm not sure if it fits your explanation regarding if the bracket is the first thing in a line:

a = [
b( ...
c, ...
d ...
)
];

format once:

a = [
     b( ...
  c, ...
  d ...
 )
    ];

twice:

a = [
     b( ...
       c, ...
       d ...
      )
    ];

Yet the following code will only need once:

            b( ...
        c, ...
    d ...
);

[florianschanda]

Yep, same thing, same issue. So there is three ways of fixing this, all with immense down-sides:

• The clean option, via mh_rewrite. But this tool will be a major effort to write.
• The nasty option, by basically parsing and fixing again until we get to a fix-point. Easy to do but oh dear is it hideous. Also slow.
• The ugly option, by somehow making another hack over the existing layer of hacks to make this work. This is what I looked at so far, but I think without re-thinking somehow how the fixer works it can't be done safely.

That said, I have totally re-written the fixer one before, so maybe it's time to re-design again with this kind of issue in mind.

I'll file the test-cases you made (thanks btw) and will keep this open, but nothing will happen this year pretty sure.

[florianschanda]

Maybe I will do option 2, but as an additional option (i.e. non-default behaviour), just in case somebody is willing to pay the performance hit.