More code folding issues

[rrousselGit]

This is a follow-up to #4121

Consider the following code:

  testSource('Does not try to parse generated providers',
      runGenerator: true, source: '''
import 'package:riverpod/riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';

part 'foo.g.dart';

final legacy = Provider<int>((ref) => 0);

@riverpod
int simple(SimpleRef ref) => 0;

// Regression test for https://github.com/rrousselGit/riverpod/issues/2194
@riverpod
int complex(ComplexRef ref, {int? id, String? another}) => 0;
''', (resolver) async {
    final result = await resolver.resolveRiverpodAnalysisResult();

    expect(result.legacyProviderDeclarations, hasLength(1));
    expect(result.generatorProviderDeclarations, hasLength(2));
  });

Collapsing testSource leads to:

Screen.Recording.2024-02-11.at.09.17.05.mov

[DanTup]

The folding range for the function call (f) seems to be being cut short:

Not sure if it might be related to dart-lang/sdk@604bf2f which avoids two folding ranges ending on the same line because VS Code doesn't handle them. I would expect that would only reduce the range by a single line though, and this looks way off.

[DanTup]

Ok, so this is caused by the change above. It's working as it was intended to, but it does result in this behaviour.

The issue is that VS Code only does "line folding", it will ignore the characters we provide. It will not allow any folding ranges to intersect or start/end on the same lines. So given this code:

  f(
    'string',
    () {
  });

We will try to produce two ranges (one around all of the arguments, and one for the body of the function). Since they end on the same line, we chop the first one and make it end before the second one starts.

I don't think we have any good options here. We could:

1. Do what we're doing - truncate the first range to end before the second one starts
2. Drop the second range entirely (this would prevent the function from being foldable)
3. Reduce the second range to end on the line before to avoid it conflicting

I suspect in this case (3) might be slightly better, but I don't think that's always the case.

Let's leave this open to collect feedback and if this kind of code is common we could try the other way (or do do something smarter).

[rrousselGit]

3. Reduce the second range to end on the line before to avoid it conflicting

I suspect in this case (3) might be slightly better, but I don't think that's always the case.

I think that makes total sense.
To me, it feels very weird that the button to collapse the closure is not in the line where the closure starts.

[DanTup]

To me, it feels very weird that the button to collapse the closure is not in the line where the closure starts.

I'm not sure I understand this - in both (1) and (3) the starts of the ranges should not change, only the ends. I'm not sure where the closure fold is starting in the wrong place.

[rrousselGit]

Sorry, I got confused.
I better understand the problem after re-reading. That explains why adding a trailling comma after the closure fixes the issue.

I assume this is mainly impacting tests.
Tests already have unique formatting, such as not requiring a trailling comma in the current scenario. Maybe an alternative is to implement 2., but only for @isTest/@isGroup functions?

[rrousselGit]

A less specific scenario would be:

  test(
      'Hello '
      'World', () {
    print('foo');
  });