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

Use super parameters in snippets #3899

Closed
guidezpl opened this issue Apr 1, 2022 · 8 comments
Closed

Use super parameters in snippets #3899

guidezpl opened this issue Apr 1, 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 enhancement
Milestone

Comments

@guidezpl
Copy link
Contributor

guidezpl commented Apr 1, 2022

Is your feature request related to a problem? Please describe.
Feature: dart-lang/language#1855, Flutter repo work: flutter/flutter#100575

Describe the solution you'd like
Tweak the snippets to adopt this feature

@DanTup
Copy link
Member

DanTup commented Apr 1, 2022

@bwilkerson for code generated by the server (for ex. snippets), should super parameters be used whenever supported, or only if the lint is also enabled?

@DanTup DanTup added this to the v3.39.0 milestone Apr 1, 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 Apr 1, 2022
@bwilkerson
Copy link

I think it's reasonable for snippets to make use of any valid language feature that makes the generated code better.

I think that it's reasonable to not produce some style of code when there's a lint that prohibits it (that is, to allow lints to reduce the set of valid language features), but I don't think we want to require users to "enable" new language features before we start producing them in generated code. Or, at least, we don't want to require them to enable it again; they already had to opt in once by increasing the minimum SDK version.

@goderbauer
Copy link

+1 I think if the package has a lower SDK bound of at least 2.17.0-0 (indicating that super params are supported) the macros should use them.

@DanTup
Copy link
Member

DanTup commented Apr 5, 2022

Working on a change at https://dart-review.googlesource.com/c/sdk/+/240281/.

It will only work for server-provided snippets, which are currently something you can turn on if on Flutter master using the dart.enableServerSnippets setting but will automatically be enabled when your SDK supports them in an upcoming release of the VS Code extension.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Apr 6, 2022
Fixes Dart-Code/Dart-Code#3899.

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

DanTup commented Apr 6, 2022

Landed in dart-lang/sdk@fa024be.

@DanTup DanTup closed this as completed Apr 6, 2022
@eseidelGoogle
Copy link

Re: snippets, do https://github.com/Dart-Code/Dart-Code/blob/master/snippets/flutter.json also need to change?

@DanTup
Copy link
Member

DanTup commented May 2, 2022

@eseidelGoogle those snippets shipped in the extension cannot adopt to language versions, so updating them will result in errors for anyone on the current stable SDKs (which don't support super params). Those snippets are being phased out - newer Dart/Flutter SDKs (not the current stable version, but future versions) will get their snippets from the server, which can adopt to language versions (and lints).

We'll automatically switch to the server snippets when they're available (any version of Dart >= 2.17.0-258), so if you're using master you should already see super params in the snippets. If you're on stable, it'll happen automatically after the next stable release.

Those local snippets will likely be removed some time in the future when we drop support for old SDK versions (there's no planned timeline, but ultimately we'll require versions of the SDK that support LSP and DAP, and you'll need to use an older VS Code extension version if you really need to stick on a very old Dart/Flutter version), but in the meantime they will be left as-is (to be as compatible as possible).

(lmk if anything isn't clear!)

@eseidelGoogle
Copy link

eseidelGoogle commented May 2, 2022 via email

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 enhancement
Projects
None yet
Development

No branches or pull requests

5 participants