WIP: Map workspace to iprojectsite #3391
Conversation
|
I cannot build it locally: How to fix it? |
|
@vasily-kirichenko working on it. |
Pilchie
left a comment
Pilchie
left a comment
There was a problem hiding this comment.
@jasonmalinowski it would be great if you could also look at this.
| | WorkspaceChangeKind.AdditionalDocumentAdded | ||
| | WorkspaceChangeKind.AdditionalDocumentRemoved | ||
| | WorkspaceChangeKind.DocumentInfoChanged | ||
| | WorkspaceChangeKind.SolutionCleared -> |
There was a problem hiding this comment.
Do you need SolutionAdded/SolutionRemoved here too?
| let workspaceChanged (args:WorkspaceChangeEventArgs) = | ||
| match args.Kind with | ||
| | WorkspaceChangeKind.ProjectAdded | ||
| | WorkspaceChangeKind.ProjectChanged |
There was a problem hiding this comment.
The event tells you the affect projects in these cases, instead of needing to iterate all projects
There was a problem hiding this comment.
@Pilchie I have had it both ways. I will go back to per project ... it seems like less overhead.
| let p = workspace.ProjectTracker.GetProject projectId | ||
| match p with | ||
| | null -> | ||
| true, projectContextFactory.CreateProjectContext( |
There was a problem hiding this comment.
Some comments about the fact that the language service is the one responsible for putting legacy projects into the workspace, but the project system does it for CPS based projects might make this clearer.
At first glance it was strange that this existed in a method that is hooked up to WorkspaceChanged events. It wasn't clear how there could ever be projects that would be null here.
|
|
||
| for referencedSite in referencedProjectSites do | ||
| let referencedProjectId = setup referencedSite | ||
| | _ -> () |
There was a problem hiding this comment.
I don't think this is reachable - we don't have any implementations of IWorkspaceProjectContext that aren't AbstractProject.
There was a problem hiding this comment.
@Pilchie I don't think the compiler knows that, and that may also change in the future.
There was a problem hiding this comment.
Right - I forgot F# pattern matches were complete.
| [| | ||
| for reference in project.ProjectReferences do | ||
| let p = workspace.CurrentSolution.GetProject(reference.ProjectId) | ||
| yield (p.OutputFilePath) |
There was a problem hiding this comment.
Do you want the compiler output path (usually in obj), or the final, msbuild output path (usually in bin) here?
There was a problem hiding this comment.
@Pilchie either works since we need metadata from the built dll. Final might be better if the FCS does magic resolution for dependencies not specified in the project.
| let taskReporter = None | ||
| let targetFrameworkMoniker = "" | ||
| let projectGuid () = project.Id.Id.ToString() | ||
| let creationTime = System.DateTime.Now |
There was a problem hiding this comment.
Consider using UTC - this could go backwards if VS is running when the timezone/DST of the machine changes.
|
|
||
| member private __.ProvideProjectSiteProvider(workspace:Workspace, project:Project) = | ||
| let visualStudioWorkspace = workspace :?> VisualStudioWorkspace | ||
| let hier = visualStudioWorkspace.GetHierarchy(project.Id) |
There was a problem hiding this comment.
Consider handling null, particularly for Lightweight solution load.
|
Also tagging @srivatsn here. |
| {new IProvideProjectSite with | ||
| member this.GetProjectSite() = | ||
| let compileItems () = [| for document in project.Documents do yield document.FilePath |] | ||
| let compilerFlags () = [| "" |] |
There was a problem hiding this comment.
Presumably this is the piece that we'll have to address by storing the string in the Workspace here
|
closing for a pr targeting vs2017-rtm |
4 participants
Use the Roslyn workspace as a datasource for source files and referenced projects for the F# Editor language service.
Note: as of right now the source files are not in project file order.