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

Method snippet follow-up #46568

Closed
3 of 5 tasks
andrewbranch opened this issue Oct 28, 2021 · 3 comments · Fixed by #48066
Closed
3 of 5 tasks

Method snippet follow-up #46568

andrewbranch opened this issue Oct 28, 2021 · 3 comments · Fixed by #48066
Assignees
Labels
Domain: Completion Lists The issue relates to showing completion lists in an editor Fix Available A PR has been opened for this issue

Comments

@andrewbranch
Copy link
Member

andrewbranch commented Oct 28, 2021

List of follow-up items from #46370 that were easier to put in separate PR(s). I tried to sort these by priority order, and the last couple I don’t think we’ll schedule at all for now. We can close this issue when we’ve done everything worth scheduling for a milestone.

  • Add $0 tabstop at the same location as $1 tabstop (in method body)

  • Make auto-imported types work

    • Moderate effort
    • Sending this back as part of getCompletionsAtPosition would require a protocol change and corresponding update in typescript-language-features. There are some workarounds we could implement by sending the back as part of getCompletionEntryDetails instead, which would work with no VS Code update. Maybe we should discuss this at next week's editor sync. My feeling is this qualifies as a bug fix, but if we need a VS Code update, we’ll need to coordinate with @mjbvz, and we’re also nearing the end of our own release cycle, so I’m not sure what the best strategy is here.
  • From @amcasey at Add method signature completions #46370 (comment): public must precede abstract.

    • Maybe 4.6?
  • From @amcasey at Add method signature completions #46370 (comment):

    if you name the method to be overridden "abstract", "async", etc, the completion clobbers the keyword completion. It did this before the change too, but then it would insert the same text either way - now it inserts a whole function declaration.

    • This can be backlog / help wanted / when you have time for it.
  • Improve addMissingMember codefix’s generated implementation signature for overloads (currently all parameters are unknown (as of feat(47281): Implementing an interface suggests any as type #47308))

    • Let's listen for feedback about this. Unless you feel strongly about it or we hear a lot of complaints, I think this can be backlog / help wanted.
@andrewbranch
Copy link
Member Author

andrewbranch commented Oct 29, 2021

Two more things:

  • Get rid of empty replacementSpan Add method signature completions #46370 (comment)
  • Maybe show more than just the name in the completion list; I think a display of method(p1: string, p2: number, ... (probably the whole first line of the insertText, maybe truncated at some number of characters?) would be better, but we can discuss this at an upcoming editor sync.

@gabritto
Copy link
Member

  • Maybe show more than just the name in the completion list; I think a display of method(p1: string, p2: number, ... (probably the whole first line of the insertText, maybe truncated at some number of characters?) would be better, but we can discuss this at an upcoming editor sync.

I have found I don't miss this when I have completions preview turned on. Also, at least in VSCode there is a truncated preview of the insert text next to the name of the completion, but I don't know how other editors display this.

@andrewbranch
Copy link
Member Author

I have found I don't miss this when I have completions preview turned on

100%. I think this is why I didn't notice this in my initial review of #46370. But it's still not the default.

Also, at least in VSCode there is a truncated preview of the insert text next to the name of the completion

This is actually the detail text that we’ve always generated in completion details. It does help, but the fact that it's the same as it's always been for completions that just insert the name (and the same as it will continue to be if a user disables the member completion snippets) makes me feel like maybe it should be more obvious that we're going to insert additional text... but this is definitely worth gathering feedback on; I don't feel like I have the perfect answer.

@gabritto gabritto added the Domain: Completion Lists The issue relates to showing completion lists in an editor label Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Completion Lists The issue relates to showing completion lists in an editor Fix Available A PR has been opened for this issue
Projects
None yet
4 participants