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

Productivity: Completion should have a way to see items there is no using/reference for #7053

Closed
Pilchie opened this issue Nov 25, 2015 · 79 comments

Comments

@Pilchie
Copy link
Member

Pilchie commented Nov 25, 2015

No description provided.

@dfkeenan
Copy link

This would be pretty good. For example extension methods. I always for get to add the using statement for "System.Data.Entity" which gives you a bunch more extension methods for Entity Framework.

I think it definitely needs some visual cue as to whether or not a using and/or reference needs to be added. Also need to be careful about how much is included. With fuzzy matching you could include a whole bunch of symbols which might be more confusing than helpful. I am not sure how the filtering/fuzzy matching is done. Some sort of distance function i.e. Levenshtein distance? But for symbols that are not already accessible there should probably be some minimum match quality before a symbol is included. Maybe this could be user configurable.

Or am I just making things more complicated than they need to be??

@Pilchie
Copy link
Member Author

Pilchie commented May 10, 2016

@dfkeenan I think we'll definitely have to be careful, and provide some indication that a significant operation (adding a nuget package, reference, or using) is also going to happen. I'm less excited about the idea of making the match quality a user configurable thing though...

@dfkeenan
Copy link

@Pilchie I was just throwing ideas out there. As long as the match quality threshold is well balanced there is no need to make it user configurable.

@jankratochvilcz
Copy link

Would love to see this bumped :-). After not using Resharper for a month now, this is definitely the most important feature I miss, it speeds up writing code a ton!

@sunxinzhe2012
Copy link

@DustinCampbell this feature is so important for us, nearly two years since this issues created,do you have any plans? so thanks!

@DustinCampbell
Copy link
Member

@Pilchie, @kuhlenh: We made several IntelliSense improvements for D15 and D15.3. Is this moving further up our list now?

@sunxinzhe2012
Copy link

@DustinCampbell @Pilchie @kuhlenh any news?cry cry cry......

@kuhlenh kuhlenh modified the milestones: 16.0, Unknown Jul 12, 2017
@kuhlenh
Copy link

kuhlenh commented Jul 12, 2017

@sunxinzhe2012 added the milestone. This is definitely near the top of our backlog :)

@sunxinzhe2012
Copy link

@kuhlenh oh yeah,good job,tk u and your team!

@sujaysarma
Copy link

+1 on this as per mail thread.

@rchande
Copy link
Contributor

rchande commented Oct 3, 2017

Thoughts:

  • Add Using builds up some table of type -> namespace. We would probably want completion to pull from the same data. I'm not sure if that table is built up incrementally and stored forever, or if we only add entries when Add Using requests the namespace for a missing type.
  • To avoid showing a massive and cluttered list, we might want to add items without usings once there are no filter matches (or something). We don't currently have a mechanism for doing this. Eg, list starts out showing only locals, the user doesn't find a match and we retrigger the list to include locals and class member

@clairernovotny
Copy link

Is there anything that can be done to get this pulled into 15.5 or another 15.x release? This is the last major thing I use ReSharper for...

@CyrusNajmabadi
Copy link
Member

@rchande

Add Using builds up some table of type -> namespace

I don't think such a table would be necessary. We could certainly just populate the list with the known typenames that the AssemblySymbol already exposes. Then, we could lazily find that type for the description when it is shown.

Now, if we want to also show the import that will be added, we would need to know the containing namespace for the type.

This is not currently easily exposed, but it could be if we update the declaration-table at the compiler layer appropriately. i.e. the compiler knows exactly what namespace the type is in when it builds the TypeNames collection. We could have a TypeName-to-Container map that gets created as well and exposed from assembly symbols.

The benefit here is that this is already in the declaration table, is extremely cheap, and is easy to keep up to date. Importantly, it does not cause symbols to be created, and it does not require walking the entire symbol.

@CyrusNajmabadi
Copy link
Member

To avoid showing a massive and cluttered list, we might want to add items without usings once there are no filter matches

That. Or there could be a new chord that creates this sort of list. i.e. instead of ctrl-space, ctrl-shift-space.

Also, i'm skeptical that "a massive and cluttered list" is actually a problem. Today, it's easy to get into a situation where you have a dozen or so imports, and the list already contains 10k+ entries. At this point the list is already massive and cluttered. So having more entries doesn't really change anything. I think what is relevant is that the list not feel cluttered once the user starts filtering things down. And that will take a bit of experimentation to see if there actually is an issue or not. My gut tells me 'no'. But we'd need to see.

@clairernovotny
Copy link

clairernovotny commented Oct 11, 2017

I would not want an extra chord. I like the way R# presents the types that need an import -- the types are there:

FrobModel (MyProduct.Models). The parentheses only exists for types that are not in scope, so it's obvious. That also solves the issue for when a type might be in different namespaces, since you'd see an entry per namespace. The namespace is not used in camelCase/PascalCase filtering, it's just for display.

I don't think type pollution would be a problem.

Limiting to only being after the filter is expired also impedes discovery. The whole point is that I don't know/fully remember the type name (or am too lazy to use the import).

The completion should add the using statement too (if that wasn't already obvious).

@CyrusNajmabadi
Copy link
Member

Limiting to only being after the filter is expired also impedes discovery.

Discovery seems hard no matter what. Your list will have 100s of thousands of items in it. How do you discover anything of value in that, unless you are filtering down to things you think are are relevant?

@Dreamescaper
Copy link
Contributor

One concern after testing PR from @genlu .
Currently all completion items are sorted alphabetically - both unimported and regular ones. And it makes regular items of much lower priority.
E.g. if I have property ListOfNames, and I start typing Li, I see bunch of items like LicenceSomething from some unimported namespace. So I need to type much more to have target item pre-selected or even visible.
I would expect that regular items have higher priority, than unimported ones - probably they should be sorted after regular ones, not sure if there's other options.

wdyt?

@clairernovotny
Copy link

I agree that in scope items should be before the unimported ones. And the namespace of the unimported one should be in parenthesis of the list to show where it’s coming from.

@genlu
Copy link
Member

genlu commented Apr 15, 2019

@Dreamescaper Thanks for the feedback! And I agree with you, we need to prioritize in-scope symbols in completion list to make it usable. I will fix that. Other other hand, have you noticed any perf regression in typing (which has been the main focus of the PR, so I'm curious)?

@onovotny Are you suggesting to add parenthesis to the greyed-out namespace text?

image

@clairernovotny
Copy link

@onovotny Are you suggesting to add parenthesis to the greyed-out namespace text?

Yes, for two reasons:

  1. I think it may help make it more clear that it's not currently in scope. It's a bigger visual indicator, especially when the completion window is narrower.
  2. That's what ReSharper does, likley for the prior reason

@genlu
Copy link
Member

genlu commented Apr 15, 2019

OK, I will take your feedback in to account. Thanks! :)

@genlu
Copy link
Member

genlu commented Apr 17, 2019

Completion for unimported types is implemented by #34808. Currently we only have an option to enable/disable it in completion list. New short cut OR new filter will come in a later update (but not both, and we are still debating internally which way to go).

@genlu
Copy link
Member

genlu commented Apr 17, 2019

@onovotny FYI I didn't add parenthesis to namespace in current implementation. Will wait for more feedbacks until making a final decision.

@jnm2
Copy link
Contributor

jnm2 commented Apr 17, 2019

@genlu If you don't prefer parentheses, what about something like +System.Collections.Immutable to make it clearer that autocompleting that item will be adding a using directive?

@genlu genlu removed the Need Design Review The end user experience design needs to be reviewed and approved. label Apr 17, 2019
@genlu
Copy link
Member

genlu commented Apr 17, 2019

@jnm2 I thin that's reasonable too

@Dreamescaper
Copy link
Contributor

@genlu
Found minor issue with type completions. Used sort text ("~{0}~{1}") makes type with shorter name go after longer one, e.g. now 'SomeTypeWithLongerName' < 'SomeType'.
Probably smth like "~{0} {1}" or "~{0}!{1}" should be used.

@genlu
Copy link
Member

genlu commented Apr 18, 2019

@Dreamescaper Ah, I see. Will fix. Thanks!

@Dreamescaper
Copy link
Contributor

Dreamescaper commented Apr 19, 2019

Found another usability issue.
For some reason it tends to pre-select import completions over regular ones.
image

@CyrusNajmabadi
Copy link
Member

Great catch @Dreamescaper . Def something we'll want to look into improving @genlu . Completion items rules have a match priority. We might want to make these negative to indicate they have less priority than a match with any normal (0) item.

@genlu
Copy link
Member

genlu commented Apr 20, 2019

@CyrusNajmabadi Making the match priority for import items lower probably wouldn't help in this particular case, I think the reason that the import item is considered a better match is because it doesn't have the <> suffix, CompareMatch uses the entire display text which includes the suffix.

@genlu
Copy link
Member

genlu commented Apr 20, 2019

@Dreamescaper I have created a bug to track this #35163. Thanks!

@CyrusNajmabadi
Copy link
Member

CompareMatch uses the entire display text which includes the suffix.

That seems strange to me. We likely need to rethink some of the logic here :)

@genlu
Copy link
Member

genlu commented Apr 23, 2019

Since the change for type only import completion was merged in #34808, I'm closing this long running issue. #35059 and #35060 are used to track the remaining feature work. Feel free to comment on this issue or open new issues if you have any feedback for this feature. Thanks!

@genlu genlu closed this as completed Apr 23, 2019
@YairHalberstadt
Copy link
Contributor

I started using this today and it's fantastic!

Thanks for all your hard work!

@chucker
Copy link

chucker commented May 22, 2019

Pretty cool indeed. Should we file problems we encounter as separate issues?

@genlu
Copy link
Member

genlu commented May 22, 2019

@YairHalberstadt @chucker
Thanks for trying this new feature out :)
And yes, creating new issues for any feedback/bug is preferred, but I will keep an eye on this thread as well.

@oliverjanik
Copy link

How do I enable this feature? What version of VS will this be in?

@chucker
Copy link

chucker commented Jun 28, 2019

@oliverjanik it's shipping (as opt-in) in VS 2019 16.1. https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#net-productivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests