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

Autocomplete for string-indexed members even from dot-notation #36429

Closed
JasonKleban opened this issue Oct 17, 2017 · 7 comments
Closed

Autocomplete for string-indexed members even from dot-notation #36429

JasonKleban opened this issue Oct 17, 2017 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality javascript JavaScript support issues typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@JasonKleban
Copy link

Sometimes the keys of a map-object are not valid typescript identifiers, yet they are strongly typed members still. Invoking autocomplete within [''] does suggest them and, as you know, the string indexer is fully type-checked in such a situation.

Supposing I have members hello, return and delete on an object, can they be suggested in the dot-notation autocomplete as well and then just automatically converted to indexer form since?

myObj.            // should offer all members, `hello`, `return` and `delete`
myObj['return']   // choosing `return` replaced the dot-notation with indexed notation and completes the accessor
@vscodebot vscodebot bot added the typescript Typescript support issues label Oct 17, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented Oct 20, 2017

You should be able invoke autocomplete inside of [''] using ctrlspace

screen shot 2017-10-19 at 8 14 04 pm

We just don't show quick suggestions automatically in this case because you are inside of a string. Does this address the issue?

@mjbvz mjbvz added the info-needed Issue requires more information from poster label Oct 20, 2017
@JasonKleban
Copy link
Author

@mjbvz - thanks for the look and the suggestion, but it doesn’t satisfy my interest. Part of autocomplete’s value is in exploring unfamiliar apis and even needing to know that there are indexed members is a burden - it’s a little more awkward and we have to check both lists.

In my case, I’m generating Rest client code off swagger definitions, organized by route template and http method. Rather than munging those strings into identifiers and worrying about collisions among them and having the consumer deal with the munged repesentations of the routes, I’m leaving the templates intact:

service[‘order/{orderid}’][‘delete’](35552)

I really like this except I give up a bit in discoverability.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 20, 2017

I don't think I understand the issue. In your service example, can you please explain the expected behavior compared to the actual behavior today?

@JasonKleban
Copy link
Author

A user is not used to typing [' for member access.

They might explore the api by typing service. and get nothing or perhaps some hasOwnProperty and toString kind of suggestions. Or depending on the generation, then might get suggestions for identifier-compatible names for completing expressions like service.orders.get() and yet not see the others because they are not compatible with the dot-notation. This is confusing - fixing it would improve the discoverability of such an api. I can't think of any negative consequences for providing string-indexed suggestions too.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 20, 2017

Ok, so my understanding is:

Repo

  1. For the code:
const foo = {
    'string': 'abc',
    'spacey string': 'abc'
};

foo.
  1. Trigger completions after foo.

Expected
Shows completions for both string and spacey string. Selecting spacey string should complete to:

foo['spacey string']

Current
Only a completion for string is shown:

screen shot 2017-10-20 at 11 27 01 am

Is that correct?

@JasonKleban
Copy link
Author

Yes. Thanks.

@mjbvz mjbvz added feature-request Request for new features or functionality javascript JavaScript support issues and removed info-needed Issue requires more information from poster labels Oct 20, 2017
@mjbvz mjbvz added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Oct 23, 2017
@mjbvz mjbvz added this to the December 2017/January 2018 milestone Jan 9, 2018
@mjbvz mjbvz closed this as completed in 5fe3f97 Jan 9, 2018
@mjbvz mjbvz added the verification-needed Verification of issue is requested label Jan 9, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Jan 9, 2018

Fixed on the VSCode side but requires a build of typescript with microsoft/TypeScript#20547 . Should be in 2.7 final. Once the TS PR is merged you can also pick it up by installing typescript@next into your workspace

@roblourens roblourens added the verified Verification succeeded label Feb 1, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality javascript JavaScript support issues typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants