Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-indentation on ENTER only works in { braced } blocks #5889

Open
geekley opened this issue Jul 4, 2023 · 7 comments
Open

Auto-indentation on ENTER only works in { braced } blocks #5889

geekley opened this issue Jul 4, 2023 · 7 comments

Comments

@geekley
Copy link

geekley commented Jul 4, 2023

Environment data

VS Code version: 1.79.2
C# Extension version: 1.25.7
⚠️ Actually in this extension which is a fork of the official extension; but presumably it happens here too (please confirm).

OmniSharp using mono: 6.12.0

Dotnet Information
.NET SDK (reflecting any global.json):
 Version:   6.0.119
 Commit:    044cde2ce0

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  23.04
 OS Platform: Linux
 RID:         ubuntu.23.04-x64
 Base Path:   /usr/lib/dotnet/sdk/6.0.119/

global.json file:
  Not found

Host:
  Version:      6.0.19
  Architecture: x64
  Commit:       e37fab9fc9

.NET SDKs installed:
  6.0.119 [/usr/lib/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.19 [/usr/lib/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.19 [/usr/lib/dotnet/shared/Microsoft.NETCore.App]

Settings

  • "C# formatter" setting was disabled, but enabling it (and restarting the IDE) didn't make any difference. It seems the formatter works only when I explicitly call it, not when typing ENTER.
  • "Format on type" setting is enabled, but "format on save" is not.

Steps to reproduce

  1. Make sure "editor.autoIndent": "full" is set for C#
  2. Optionally create .editorconfig with:
root = true
[*.cs]
indent_style = space
indent_size = 2
csharp_indent_case_contents = true
csharp_indent_switch_labels = false
  1. Create A.cs file on VSCode
  2. Paste the code below and press ENTER after each code line to check how indentation is bad when you don't use braces. It's going to where I put the comments.
class A {
  // CORRECT: Auto-indented here
  internal void f() =>
  // BAD: Should probably increase indentation here
    f(false);
    // WRONG: Did not decrease indentation
  void f(bool b) {
    // CORRECT: Auto-indented here
    if (b) {
      // CORRECT: Auto-indented here
    }
    if (b)
    // WRONG: Did not increase indentation
      f(!b);
      // WRONG: Did not decrease indentation
    else
    // WRONG: Did not increase indentation
      f(b);
      // WRONG: Did not decrease indentation
    for (; b; b = !b)
    // WRONG: Did not increase indentation
      f(b);
      // WRONG: Did not decrease indentation
    while (b)
    // WRONG: Did not increase indentation
      f(b = !b);
      // WRONG: Did not decrease indentation
    switch (b.GetHashCode()) {
      // ACCEPTABLE: But I have `csharp_indent_switch_labels = false` in .editorconfig
      case 0:
      // ACCEPTABLE: But I have `csharp_indent_case_contents = true` in .editorconfig
      break;
      // CORRECT: Kept indentation here
      case 1:
        break;
        // BAD: Ideally, smart analysis could revert to label indentation here
    case 2:
    // WRONG: Did not increase indentation
    return;
    // CORRECT: Kept indentation here
    default:
      return;
      // BAD: Ideally, smart analysis could revert to label indentation here
    }
  }
}

Expected behavior

It should auto-add indentation properly when you press ENTER, as described in the code above.

Actual behavior

Indentation is as described above, which gives you a really bad coding experience (specially if you indent with spaces).

Additional context

Note that when you hover the folding regions, only the braces generate folding. There is no folding for non-braced statements like if, for, etc. This could be related to why the editor is not indenting correctly. But it's also happening on the function with expression body despite it having folding, so maybe not.

I don't know if this should be implemented through language defined brackets, onEnterRules or indentationRules. In any case, if you want to do a "smart" analysis, you could probably improve indentation even inside a switch block, so after a "top-level" break, continue, goto or return statement, it would revert to label indentation.

@arunchndr arunchndr added this to the GA milestone Jul 6, 2023
@arunchndr arunchndr added the Bug label Jul 6, 2023
@sharwell
Copy link
Member

sharwell commented Jul 7, 2023

@arkalyanms The VS Code team is not interested in discussing this matter with us.
image

@sharwell sharwell assigned arunchndr and unassigned sharwell Jul 7, 2023
@arunchndr arunchndr removed this from the GA milestone Jul 7, 2023
@JoeRobich
Copy link
Member

Maybe this could be brought up at the LSP meeting since we were previously requesting a new LSP API. This has come up a few times before and it seemed to be contingent on microsoft/vscode#13960 being implemented.

@arunchndr
Copy link
Member

Maybe this could be brought up at the LSP meeting since we were previously requesting a new LSP API. This has come up a few times before and it seemed to be contingent on microsoft/vscode#13960 being implemented.

Yes thats my plan too.

@geekley
Copy link
Author

geekley commented Jul 7, 2023

it seemed to be contingent on virtual space being implemented.

Sorry if I misunderstood, but I believe this has no dependency on virtual space.
Other languages like JS/TS currently already do this just fine in VSCode - the indentation goes where it should go regardless of braces - though it's not perfect (e.g., it doesn't unindent after the unbraced statement line, even with ;).

@JoeRobich
Copy link
Member

@geekley To be fair to the C# extension, your scenarios fail in ts/js with FormatOnType disabled. Which makes sense because indentation is currently determined based on open brackets and they have a similar set of brackets.

JS/TS example
class A {
  // CORRECT: Auto-indented here
  l = () =>
    // BAD: Should probably increase indentation here
    f(false);
  // WRONG: Did not decrease indentation
  f(arg) {
    // CORRECT: Auto-indented here
    if (b) {
      // CORRECT: Auto-indented here
    }
    if (b)
      // WRONG: Did not increase indentation
      this.f(!b);
    // WRONG: Did not decrease indentation
    else
      // WRONG: Did not increase indentation
      this.f(b);
    // WRONG: Did not decrease indentation
    for (; b; b = !b)
      // WRONG: Did not increase indentation
      this.f(b);
    // WRONG: Did not decrease indentation
    while (b)
      // WRONG: Did not increase indentation
      this.f(b = !b);
    // WRONG: Did not decrease indentation          
  }
}

@JoeRobich
Copy link
Member

JoeRobich commented Jul 7, 2023

Ah, actually I see that geekley had FormatOnType enabled. JS/TS does much better in that scenario. O# doesn't perform any formatting on Enter. Which brings me to this old PR which is where I remembered the discussion around virtual space.

@geekley
Copy link
Author

geekley commented Jul 7, 2023

You pinged someone else lol.

Yes, I just tested here and you're right. Interesting. So that behavior is provided by a formatter? It seems JS/TS completely ignores editor.autoIndent setting, because it does indentation "right" even if you set it to keep or none.

But I believe VSCode extensions can still provide this independently of formatOnType, right? Otherwise there would be no point to the autoIndent setting. In any case, my main point is that this feature is orthogonal to virtual space availability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants