Skip to content

Conversation

@dkattan
Copy link
Contributor

@dkattandkattan commented Feb 13, 2024

PR Summary

Removes artificial Uri schema restriction, provides a mechanism for language clients to specify URIs that direct to PSProviders

PR Context

@dkattandkattan marked this pull request as ready for review February 14, 2024 19:55
@dkattandkattan requested a review from a teamFebruary 14, 2024 19:55
@dkattan
Copy link
ContributorAuthor

@andyleejordan Curious your thoughts on this. It ended up being heavier-handed than I wanted with all the async changes, but I'm wondering if that would be desirable if I split it into a separate MR? I know @SeeminglyScience doesn't want me changing the public interfaces, but they likely should've been Task<> from the start, async or not.

@dkattandkattan changed the title Asyncify WorkspaceService/Perform File IO operations through PowerShellReplaced WorkspaceFileSystemWrapper with Get-Content and Get-ChildItemFeb 15, 2024
@dkattandkattan changed the title Replaced WorkspaceFileSystemWrapper with Get-Content and Get-ChildItemDraft: Replaced WorkspaceFileSystemWrapper with Get-Content and Get-ChildItemFeb 15, 2024
@dkattandkattan marked this pull request as draft February 15, 2024 21:36
@dkattandkattan changed the title Draft: Replaced WorkspaceFileSystemWrapper with Get-Content and Get-ChildItemReplaced WorkspaceFileSystemWrapper with Get-Content and Get-ChildItemFeb 15, 2024
@SeeminglyScience
Copy link
Collaborator

I know @SeeminglyScience doesn't want me changing the public interfaces, but they likely should've been Task<> from the start, async or not.

Can you talk through what you're trying to solve? When you make a synchronous method return Task<> all you're really doing is requiring wrapper object be created that will be immediately discarded. It doesn't change the behavior at all if that's the assumption.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dkattan@SeeminglyScience