-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Completion for #:project paths (round 2)
#80844
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
Completion for #:project paths (round 2)
#80844
Conversation
|
|
||
| // TODO: Span-ify GetDirectoryName and similar helpers | ||
| var contentDirectory = PathUtilities.GetDirectoryName(contentPrefix.ToString()); | ||
| var items = await fileSystemHelper.GetItemsAsync(contentDirectory, context.CancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it supported to reference projects in other directories? For example, should we consider adding all the projects in the solution to this set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could definitely consider providing completions for "projects we know about". That might be more convenient than simply recommending "here's the next directory in the path you are typing out".
Since we have a Document, could we literally go from Document to Solution, get all the project paths, and include relative paths to the projects, as completion items?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup that sounds like what I was thinking. Have to be careful to make sure the project path exists (there are occasionally virtual projects for various edge scenarios - misc, typescript, razor)
That might be more convenient than simply recommending "here's the next directory in the path you are typing out"
I think both are definitely valuable (may not always have a solution open)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take a stab at it in a follow up
| [".csproj", ".vbproj"], | ||
| CompletionItemRules.Default); | ||
|
|
||
| // TODO: Span-ify GetDirectoryName and similar helpers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this suggestion is not straightforward and that the benefit is somewhat speculative. I would prefer to not put a tracking issue for it on the backlog, so I am going to just delete the comment.
| // TODO: Span-ify GetDirectoryName and similar helpers |
| { | ||
| for (var i = 0; i < span.Length; i++) | ||
| { | ||
| if (!char.IsWhiteSpace(span[i])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider just unifiying this helper with
roslyn/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/StringExtensions.cs
Line 14 in ce54e69
| public static int? GetFirstNonWhitespaceOffset(this string line) |
jasonmalinowski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that we're able to reuse what we've already got here.
Replaces #79275
Related to #78755