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

Hovering over constructor's parameters shows an oversized font for the type #4120

Closed
osaxma opened this issue Aug 25, 2022 · 4 comments
Closed
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

@osaxma
Copy link

osaxma commented Aug 25, 2022

Describe the bug

Hovering over constructor parameters (named or unnamed) shows an oversized font for the type. The same oversized appears when hovering over the name of an argument of a constructor with named parameters as seen below:

Screen Shot 2022-08-25 at 11 42 49 AM

It gets a bit wild when the type is multiple typedefs (e.g. shelf's Middleware type)

Screen Shot 2022-08-25 at 11 47 11 AM

Expected behavior
I would expect the same font size as when hovering over the member definition (this is a wide screenshot so it looks smaller)

Screen Shot 2022-08-25 at 11 45 24 AM

Please complete the following information:

  • VS Code version: Version: 1.70.1
  • Dart extension version: v3.46.1
  • Dart/Flutter SDK version: Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_x64"
@osaxma osaxma added the is bug label Aug 25, 2022
@osaxma osaxma changed the title Hovering over parameters shows an oversized font for the type Hovering over constructor's parameters shows an oversized font for the type Aug 25, 2022
@DanTup
Copy link
Member

DanTup commented Aug 25, 2022

Thanks, I can repro.

class A {
  /// aaa
  final String aaa; // hover over aaa

  A({required this.aaa}); // hover over aaa
}

There's no obvious difference in the response from the server:

{"id":653,"jsonrpc":"2.0","result":{"contents":{"kind":"markdown","value":"```dart\nString aaa\n```\nType: `String`\n*package:dartcode_flutter_4118/main.dart*\n\n---\naaa"},"range":{"end":{"character":18,"line":4},"start":{"character":15,"line":4}}}}
{"id":654,"jsonrpc":"2.0","result":{"contents":{"kind":"markdown","value":"```dart\n{required String aaa}\n```\nType: `String`\n---\naaa"},"range":{"end":{"character":22,"line":6},"start":{"character":19,"line":6}}}}

There's a missing newline before the package in the first one though:

Screenshot 2022-08-25 at 10 48 55

I think either LSP client or VS Code are supposed to convert --- to a full with line, but I can't see that on the second one, so perhaps that's related (microsoft/vscode#65094?).

@DanTup
Copy link
Member

DanTup commented Aug 25, 2022

So I think there are two issues here:

  1. We need two newlines before --- for it to be rendered as a horizontal line. Otherwise it's being interpreted to mean the line before is a header. This needs fixing in the server.
  2. The missing newline before package: in the rendering may be a VS Code issue - it's rendering differently to I would expect (or GitHub does) so I've filed Newline missing in markdown rendering microsoft/vscode#159161.

Edit: This was just one issue, which was a missing \n after Type: because a single newline in Markdown is essentially ignored and the two lines still treated as a single paragraph.

@DanTup DanTup added this to the v3.50.0 milestone Aug 25, 2022
@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 25, 2022
@DanTup
Copy link
Member

DanTup commented Sep 5, 2022

Fix on the way at https://dart-review.googlesource.com/c/sdk/+/257582/.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 6, 2022
Consecutive lines in Markdown are rendered as a single paragraph. To prevent these being joined together, they need an additional newline.

Fixes Dart-Code/Dart-Code#4120.

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

DanTup commented Sep 6, 2022

Fixed by dart-lang/sdk@8bf496e. It's an SDK change, so will show up in a future SDK release.

@DanTup DanTup closed this as completed Sep 6, 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 Sep 6, 2022
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

2 participants