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

Support automatically displaying signature information, when appropriate #1132

Closed
Skylled opened this issue Aug 3, 2018 · 8 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

@Skylled
Copy link
Contributor

Skylled commented Aug 3, 2018

The new Signature Help feature triggers almost constantly, even when it doesn't make sense to, like inside of comments. Or the signature shown will be for something the next layer up.

Example code:

return ListTile(
  title: Text("Enable notifications"),
  trailing: Switch(
    value: notificationsEnabled,
    onChanged: (bool enabling){
      if (enabling) {
        // type this comma -> , and get the signature of Switch()
      } else {
        // ...
      }
    },
  ),
);
@DanTup
Copy link
Member

DanTup commented Aug 3, 2018

Ugh :(

I guess we should never trigger in comments, but maybe also limit how far up we walk trying to find an invocation. This is probably worse in Flutter because build methods tend to have a constructor invocation around huge chunks of code.

How bad does this seem? Fixing it is going to be in the SDK so I can patch in an option to turn this off, but is it bad enough that it should default to off until changes are made in the SDK and rolled into Flutter?

@DanTup DanTup added is bug in editor Relates to code editing or language features labels Aug 3, 2018
@DanTup DanTup added this to the v2.17.1 milestone Aug 3, 2018
@Skylled
Copy link
Contributor Author

Skylled commented Aug 4, 2018

I can't say how frustrating it is for others, but in my case I frequently use split view, meaning a decent portion of one view is obscured.

As long as there's an option to disable the automatic triggers, I think that would be sufficient.

@DanTup
Copy link
Member

DanTup commented Aug 4, 2018

As long as there's an option to disable the automatic triggers, I think that would be sufficient.

Good call, I forgot there was both manual and automatic triggering. I'm gonna disable automatic triggering behind a setting for now (#1134) while we figure out how to make this more reliable. Ideally we want manual triggering to work when you're deeply nested, but not trigger automatically. This may mean returning some additional data from the server to allow us to know whether the provided info is for directly where the cursor is or further up the tree.

@DanTup DanTup modified the milestones: v2.17.1, v2.18.0 Aug 4, 2018
@DanTup
Copy link
Member

DanTup commented Aug 4, 2018

v2.17.1 is out and disables automatic triggering (there's a setting to turn it back on). I'll work on making this work better in vNext if possible (leaving this issue open to track that). If you still have any issues, let me know!

@DanTup
Copy link
Member

DanTup commented Aug 23, 2018

Opened dart-lang/sdk#34241 to discuss the analyzer work. I'll work on this once there's a consensus on how best to implement.

@DanTup DanTup modified the milestones: v2.18.0, On Deck Aug 23, 2018
@DanTup DanTup added the blocked on dart / flutter Requires a change in Dart or Flutter to progress label Aug 23, 2018
@DanTup DanTup changed the title Signature Help triggers far too often Support automatically displaying signature information, when appropriate Aug 23, 2018
@DanTup
Copy link
Member

DanTup commented Aug 23, 2018

(I'm re-purposing this issue as a request to enable auto-triggering, but a non-sucky version)

@DanTup DanTup modified the milestones: On Deck, Backlog Mar 19, 2019
@DanTup DanTup added the in lsp/analysis server Something to be fixed in the Dart analysis server label Jun 1, 2020
@DanTup DanTup removed the blocked on dart / flutter Requires a change in Dart or Flutter to progress label Sep 8, 2020
@DanTup DanTup modified the milestones: Backlog, v3.15.0 Sep 8, 2020
@DanTup
Copy link
Member

DanTup commented Sep 8, 2020

Working on this in the LSP server at: https://dart-review.googlesource.com/c/sdk/+/162000

If the request for signature info is from an automatic trigger when typing ( then we'll only return results if the ( is exactly the offset of the located ArgumentList. Comma is now just a "retrigger" character, so it will only ever trigger signature help if it's already visible (and therefore returning results will just update the UI and not trigger any new popups).

@DanTup
Copy link
Member

DanTup commented Sep 22, 2020

Fixed in dart-lang/sdk@cef054a.

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