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

[CP] Fix missing closure code completion entries for function parameters #54112

Closed
DanTup opened this issue Nov 21, 2023 · 2 comments
Closed
Assignees
Labels
analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. cherry-pick-approved Label for approved cherrypick request merge-to-stable

Comments

@DanTup
Copy link
Collaborator

DanTup commented Nov 21, 2023

Commit(s) to merge

28d9ac0

Target

stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/337341

Issue Description

In the latest SDK release, completions for functions/closures are missing for LSP/VS Code:

Flutter 3.13

image

Flutter 3.16

image

This was a bug introduced in eb73dba that resulted in them not having the correct labels set and being filtered out by the client.

What is the fix

The fix is to ensure the labels for closure completions are not accidentally replaced by empty strings when cleaning up label/filterText in LSP (something done to improve the display of function names, parameters, signatures in code completion).

This is a simple change to not split completion labels (to remove parameters/etc.) if the split character is at the start of the string (this means foo() would be split to keep only foo, but () => would not result in an empty string).

Why cherry-pick

Although this is just completion and users can certainly type the function signature themselves, this feels like a significant usability regression - you'd have to hover over the parameter name to see the signature and the hover may obscure where you're typing.

This issue is being reported a lot (despite a pinned issue) 😔

It may have been reported more, but I pinned my original issue about it to try and avoid duplicates.

Risk

low

Issue link(s)

Dart-Code/Dart-Code#4837

Extra Info

(FYI @jacob314 @bwilkerson)

@DanTup DanTup added the cherry-pick-review Issue that need cherry pick triage to approve label Nov 21, 2023
@mraleph mraleph added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-server labels Nov 21, 2023
@athomas
Copy link
Member

athomas commented Nov 27, 2023

Risk looks small, has a test. +1

@athomas athomas added cherry-pick-approved Label for approved cherrypick request merge-to-stable and removed cherry-pick-review Issue that need cherry pick triage to approve labels Nov 27, 2023
copybara-service bot pushed a commit that referenced this issue Nov 28, 2023
Closure completions disappeared in eb73dba because we ended up wiping our the filterText/label when trying to clean them up for functions.

Fixes Dart-Code/Dart-Code#4837

Bug: Dart-Code/Dart-Code#4837
Change-Id: I536ce38624ee3f6047229ac58c3fe173d162079a
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/335822
Cherry-pick-request: #54112
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337341
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@athomas
Copy link
Member

athomas commented Dec 5, 2023

Released in 3.2.2.

@athomas athomas closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. cherry-pick-approved Label for approved cherrypick request merge-to-stable
Projects
None yet
Development

No branches or pull requests

6 participants