-
Notifications
You must be signed in to change notification settings - Fork 789
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
[Dev16 Preview2]Single File Management Refactor #5845
Conversation
Link #1944 |
4066c2f
to
ac0a91d
Compare
@cartermp @KevinRansom @brettfo This is ready. Nothing should regress and it should have our rename file issue resolved. There is more work to be done, however, it's important we make these changes in order to get rid of deprecated code that we are using from Roslyn. |
@TIHan and I did some extensive testing of this and think that the overall experience is good, plus it allows us to get rid of some rather fragile stuff on our end. |
} | ||
|
||
/// Get the options for a document or project relevant for syntax processing. | ||
/// Quicker then TryGetOptionsForDocumentOrProject as it doesn't need to recompute the exact project options for a script. | ||
member this.TryGetOptionsForEditingDocumentOrProject(document:Document) = | ||
let projectId = document.Project.Id |
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.
@TIHan I don't understand this change in dev16.
#10156 is a synchronous call to this on the UI thread
As the comment to this method says, the point of "TryGetOptionsForEditingDocumentOrProject" is to get editing options quickly for scripts, in the cases where we need them fast on the UI thread for editing operations. But this change removed this and now this method computed the exact options.
I believe this is a primary cause for slower script editing in dev16, e.g. #6646, and is related to the UI thread aspects of #8757
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.
Looking back, I don't think I understood this well enough at the time and made the change in hopes to normalize everything.
If this call was being invoked on the UI thread somewhere, I can totally understand why there would be a performance issue.
* Trying to fix dev16 with latest changes * Dev16 is partially working * Fixed several issues, should be working dev16 * Using try..with on computing options * Deleting unused code * Minor refactor * Check command line option cps stamps for recompute as a backup * Using ProjectId for project options * Updating binoutputpath for legacy projects to workspace project * Removing cps stamp as we don't need it * Updated roslyn package. Using IWorkspaceProjectContext.Id. * Handling single files (script files) differently * Removing dead code * Removing dead code * Removing dead code * Removing dead code * Removing dead code * Removing ProjectSitesAndFiles.fs * Initializing early to capture solution/workspace events * Handling rename of script files * Delete Fsi.nuget.props * Update Extensions.fs * Create project per script file * Make sure to dispose in fallback * Don't need that
This PR depends on this one getting merged first: #5833
This changes how we handle single files/script files. Also gets rid of the use of deprecated APIs. It's much simpler and resolves the issue when script files were re-named that all tooling would stop working for the script.