microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
722 stars 242 forks source link

If-Else statements auto-indent with each else statement #5396

Closed bjarkihall closed 4 years ago

bjarkihall commented 4 years ago

Describe the bug The AL Formatter formats if-else statements by increasing the indentation for each case. This results in e.g. nested statements to confuse those who encounter them after the formatting has taken place.

To Reproduce First start by examining 1-level deep if-else:

if p then
    a
else if q then
    b
else if r then
    c;

becomes

if p then
    a
else
    if q then
        b
    else
        if r then
            c;

Where people don't seem to agree on which on is the best practice (according to the discussion on https://github.com/MicrosoftDocs/dynamics365smb-devitpro-pb/issues/296).

But lets see what happens if we go a level further:

if p then begin
    if x then begin
        a;
    end;
end else
if q then begin
    if y then begin
        b;
    end;
end else
if r then begin
    if z then begin
        c;
    end;
end;

becomes:

if p then begin
    if x then begin
        a;
    end;
end else
    if q then begin
        if y then begin
            b;
        end;
    end else
        if r then begin
            if z then begin
                c;
            end;
        end;

Expected behavior I expected the indentation to be preserved.

p, q, r blocks should have the same indentation, as would x, y, z blocks and the a, b, c statements.

I've encountered multiple cases of if-else cases in some legacy code and would like the AL formatter to stop changing it, since I already have editor.formatOnSave set as false. The possibility to disable auto-formatting would be a solution but I wanted to discuss the indentation convention which is currently being enforced. I would think (like in other block-based programming languages) that each level should have the same indentations. The main reason this is confusing (at least for me) is because you can't let the final end; match with the initial if that started the block of code.

Versions:

bjarkihall commented 4 years ago

https://docs.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-al-control-statements This link is for reference about how the AL documentation describes if-else statements (but it doesn't go into nesting both if and else) if it's to any help.

https://docs.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-al-formatter This link is for reference about how the AL Formatter works.

bjarkihall commented 4 years ago

Duplicate of #4449 With emphasis on what happens with nesting, though, and more examples. If anyone knows how to disable the formatter temporarily (without disabling the AL extension, since that requires a restart of vscode) until this is fixed, it would be nice to know since my editor.formatOnSave setting isn't respected.

charlespockert commented 4 years ago

I happen to agree with what the formatter is doing here.

This only happens if you line break before the next conditional branch, your if statement is nested because it's not part of the else line.

If you use

if condition then begin
  // Stuff;
end else if condition then begin
  // Stuff;
end

// Or

if condition then
  // Stuff
else if condition then
  // Stuff;

Or any variant where your if statement or alternate branch lands on the same line as the else it seems to indent correctly.

I'd argue that this:

if condition then 
  // Stuff
else
if condition then
  // Stuff

Is hard to read and doesn't make much sense in terms of indentation.

bjarkihall commented 4 years ago

@charlespockert it happens also on this:

if p then begin
    if x then begin
        a;
    end;
end else if q then begin
    if y then begin
        b;
    end;
end else if r then begin
    if z then begin
        c;
    end;
end;

Which I agree is more readable. But this is still the result AL extension makes:

if p then begin
    if x then begin
        a;
    end;
end else
    if q then begin
        if y then begin
            b;
        end;
    end else
        if r then begin
            if z then begin
                c;
            end;
        end;
NKarolak commented 4 years ago

See also #4449