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

Code folding not working properly in certain circumstances #4121

Closed
andrewkolos opened this issue Aug 25, 2022 · 8 comments
Closed

Code folding not working properly in certain circumstances #4121

andrewkolos opened this issue Aug 25, 2022 · 8 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone

Comments

@andrewkolos
Copy link

andrewkolos commented Aug 25, 2022

Describe the bug

At least in top-level functions, code folding can sometimes be unavailable. For example, I might be able to fold a function signature, but not the implementation block (and some but not all blocks within). While this is difficult to describe, it is most easily observed through reproduction or the videos below.

The issue does not appear to happen in Android Studio, so I'm guessing this is not an SDK issue (though VSCode can fold parameter lists, while Android Studio cannot--I'm not sure if this is intended).

To Reproduce
Steps to reproduce the behavior:

  1. Clone https://github.com/flutter/flutter (head is f477c8b184d2364fcef0a15cee6ab5c909188229 at the time of writing)
  2. Open run_hot.dart (packages/flutter_tools/lib/src)
  3. Fold all code. This can be done by opening the command palette (Cmd/Ctrl+Shift+P) and typing Fold All.
  4. You should see multiple unfolded blocks of code. As a specific example, the body _defaultReassembleHelper won't be folded, and for loops within might not be folded as well. See screenshots below.

Expected behavior
All code blocks should be folded after issuing a Fold All command. Interestingly, copying the contents of the file into a new editor and then using Fold All appears to work as expected. However, saving the file (say, to my Desktop) will unfold the code, and if I try to Fold All again, it is still slightly broken.

Screenshots
The issue happening in run_hot.dart

2022-08-25_10-39-03.mp4

The issue not happening if I copy and paste a problematic function into a new editor:
https://user-images.githubusercontent.com/9027551/186756680-939ba825-7c52-44da-9324-b183e5add931.mp4

The issue happening if I save the new file to disk:
https://user-images.githubusercontent.com/9027551/186756837-88813e9f-1048-409e-9d83-c8e4e06bf0c5.mp4

Please complete the following information:

  • Operating System and version: macOS Monterey 12.5.1
  • VS Code version: 1.70.2
  • Dart extension version: 3.46.1
  • Dart/Flutter SDK version: 2.17.6
  • Target device (if the issue relates to Flutter debugging): N/A

I have reproduced this on Windows and another MacBook. Let me know if more information on those environments is needed.

(edit: typo)

@DanTup DanTup added in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server labels Aug 30, 2022
@DanTup DanTup added this to the v3.50.0 milestone Aug 30, 2022
@Hexagonale
Copy link

So the issue is for the functions with multiline parameters. I opened an issue for the VS Code and I think they have provided valuable debug info.

microsoft/vscode#162300

@DanTup
Copy link
Member

DanTup commented Sep 29, 2022

Thanks for the link. I didn't realise this used to work - it looks like there were some changes in VS Code (microsoft/vscode#155302) that stopp it from handling ranges that start/end on the same line.

The data from the server initially contains column numbers (so they don't really intersect at all), but VS Code only supports line folding. I think we'll need to detect this and reduce the first range to end on the line before the next one begins in this case.

@DanTup
Copy link
Member

DanTup commented Sep 29, 2022

Working on a fix at https://dart-review.googlesource.com/c/sdk/+/261900/. I'll close this once it lands. It's a change in the Dart analysis server, so will ship in a future SDK release (rather than a Dart VS Code extension release).

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 29, 2022
+ convert LSP folding tests to use TestCode parser.

A change in VS Code means two folding regions are no longer allowed to end/start on the same line (the second range is silently dropped). This truncates folding regions if they end on the same line that another starts to end on the line before (but only if a client only supports line-folding mode).

Fixes Dart-Code/Dart-Code#4121.

Change-Id: Ic26f58f84c44a01ae5157c336ed0f207d1c0eeb8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261900
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
@DanTup
Copy link
Member

DanTup commented Oct 3, 2022

Fixed by dart-lang/sdk@604bf2f.

@DanTup DanTup closed this as completed Oct 3, 2022
@DanTup DanTup added the relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available label Oct 3, 2022
@methompson
Copy link

I'm still encountering the issues described in the OP. I've installed Flutter / Dart from the beta channel and the pre-release Dart / Flutter VS Code Plugins.

  • Dart 2.19.0-255.2.beta
  • Flutter 3.4.0-34.1.pre
  • Dart-Code v3.53.20221101
  • Flutter-Code v3.53.20221101
  • VS Code 1.72.2

All running on MacOS 13.0 on a MacBook Pro 16", 2021.

Functions still minimize only on the function definition and NOT the entire function. Please note the screenshots before / after folding.
Screenshot 2022-11-01 at 11 22 38 AM
Screenshot 2022-11-01 at 11 22 44 AM

In the above screenshots, func3 and func4 should be folded the same as func1 and func2.

I'm uncertain which combination of SDKs, plugins and versions I should be utilizing to get code folding to work correctly.

@DanTup
Copy link
Member

DanTup commented Nov 1, 2022

@methompson looking at the change dart-lang/sdk@604bf2f, it looks like the first Dart SDK it was included in is 2.19.0-257.0.dev:

Screenshot 2022-11-01 at 18 38 51

The beta you're on is 2.19.0-255.2.beta so seems like it was cut just before this. Presumably the fix will be in the next beta (I actually thought there had been a more recent beta, but apparently not).

Hope this helps!

@methompson
Copy link

methompson commented Nov 1, 2022

Thanks for the insight. I've updated to the Master channel, so now I have the following installed on my system, along with the same pre-release versions of dart-code and flutter-code:

  • Dart 2.19.0-357.0.dev
  • Flutter 3.5.0-12.0.pre.56

I want to note that after installing the stable channel, I completely closed VSCode to make sure any LSPs or daemons would spawn correctly.

Attached is the current state of folding:
Screenshot 2022-11-01 at 2 01 35 PM

Screenshot 2022-11-01 at 2 01 42 PM

It seems any code block can be folded, but you cannot fold anything if a foldable code block exists within that code block. As such, there's a definite regression. Note that the class cannot be folded.

func1 can be folded, but as soon as you put a foldable block within a function, you cannot fold the function completely (evidenced by func2). The provided class cannot be folded.

Finally, here's how the Code folding looked on dart.dev:

Screenshot 2022-11-01 at 2 09 30 PM

Code used for this example as follows:

void func1(
  String a,
  int b,
) {
  print('test');
}

void func2(
  String a,
  int b,
) {
  if (true) {
    print('a');
  }
  print('test');
}

class MyClass {
  method1(String a) {
    print(a);
  }

  method2(
    String a,
    String b,
  ) {
    print(a);
  }

  method3(
    String a,
    String b,
  ) {
    if (true) {
      print(a);
    }
  }
}

@DanTup
Copy link
Member

DanTup commented Nov 3, 2022

@methompson thanks for the example! There's definitely something wrong there. I've filed #4242 with a cut-down example and will follow up there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Development

No branches or pull requests

4 participants