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 completion commit characters so ( is only inserted when used to commit #2125

Closed
vogella opened this issue Nov 25, 2019 · 3 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
Milestone

Comments

@vogella
Copy link

vogella commented Nov 25, 2019

If I code complete on "_saySomething" I get _saySomething() which the compiler immediately complains about.

    return AppBar(
          title: Text("Super application"),
          actions: [
            IconButton(
              icon: Icon(Icons.access_alarm),
              onPressed: _saySomething,
            )
          ],
        );
  }
 void _saySomething() {
    print("pressed");
  }
@DanTup
Copy link
Member

DanTup commented Nov 26, 2019

This is essentially a dupe of #687. We don't really have the context to know when a user is completing a function of whether it'll be correct to include parens on the end. The ideal solution IMO is that we never insert the parens, but if you're calling the function then you use the ( key to commit the completion (which would also insert it, and the closing one).

This is possible if you disable insertArgumentPlaceholders and turn on completionCommitCharacters, however there are other quirks in that behaviour which may make it worse (ultimately, I think that should be the end goal though).

I'll leave this open as a reminder that this still sucks (since the other issue is closed) and I'll try to work through the remaining issues that make using commit characters bad (or at least document the specifics).

@DanTup DanTup added in editor Relates to code editing or language features is bug labels Nov 26, 2019
@DanTup DanTup added this to the v3.8.0 milestone Nov 26, 2019
@DanTup DanTup modified the milestones: v3.8.0, v3.9.0 Jan 16, 2020
@DanTup DanTup modified the milestones: v3.9.0, On Deck Mar 24, 2020
@DanTup DanTup modified the milestones: On Deck, Backlog Jul 2, 2020
@DanTup DanTup changed the title Code completion creates wrong results for methods assigned to void properties Use completion commit characters so ( is only inserted when used to commit Sep 7, 2020
@DanTup DanTup modified the milestones: On Deck, v3.15.0 Sep 7, 2020
@DanTup
Copy link
Member

DanTup commented Sep 22, 2020

I've added a preview flag to enable these when using LSP, though in my current testing it doesn't seem to be working. I've opened microsoft/vscode-languageserver-node#673.

@DanTup DanTup modified the milestones: v3.15.0, v3.16.0 Sep 22, 2020
@DanTup DanTup added the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Sep 22, 2020
@DanTup DanTup added in lsp/analysis server Something to be fixed in the Dart analysis server and removed blocked on vs code / lsp / dap Requires a change in VS Code to progress labels Oct 21, 2020
@DanTup
Copy link
Member

DanTup commented Oct 21, 2020

With https://dart-review.googlesource.com/c/sdk/+/168653/, this is supported in LSP with the dart.previewCommitCharacters setting. I've opened #2895 as a reminder to enable it by-default in a future release.

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

No branches or pull requests

2 participants