-
Notifications
You must be signed in to change notification settings - Fork 420
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
Use Roslyn's editorconfig support #1771
Changes from all commits
32291fd
18fada3
140dd8d
98abdc0
9638ac8
a8ade9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,8 @@ public static void Initialize(IServiceProvider serviceProvider, CompositionHost | |
projectEventForwarder.Initialize(); | ||
var projectSystems = compositionHost.GetExports<IProjectSystem>(); | ||
|
||
workspace.EditorConfigEnabled = options.CurrentValue.FormattingOptions.EnableEditorConfigSupport; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be good to also reset this flag here https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Host/WorkspaceInitializer.cs#L62-L65 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be useful if there were a way to force all the projects to update. Is this option worth all this extra complexity? I would assume the presence of an .editorconfig would be the indicator of whether a user wanted .editorconfig support. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are right we can solve this systematically separately as there are other areas that would benefit from that |
||
|
||
foreach (var projectSystem in projectSystems) | ||
{ | ||
try | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -176,7 +176,7 @@ private async Task ProcessLoopAsync(CancellationToken cancellationToken) | |
await Task.Delay(LoopDelay, cancellationToken); | ||
ProcessQueue(cancellationToken); | ||
} | ||
catch(Exception ex) | ||
catch (Exception ex) | ||
{ | ||
_logger.LogError($"Error occurred while processing project updates: {ex}"); | ||
} | ||
|
@@ -377,6 +377,14 @@ private void WatchProjectFiles(ProjectFileInfo projectFileInfo) | |
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: true, projectFileInfo.ProjectIdInfo); | ||
}); | ||
|
||
if (_workspace.EditorConfigEnabled) | ||
{ | ||
_fileSystemWatcher.Watch(".editorconfig", (file, changeType) => | ||
{ | ||
QueueProjectUpdate(projectFileInfo.FilePath, allowAutoRestore: false, projectFileInfo.ProjectIdInfo); | ||
}); | ||
} | ||
|
||
if (projectFileInfo.RuleSet?.FilePath != null) | ||
{ | ||
_fileSystemWatcher.Watch(projectFileInfo.RuleSet.FilePath, (file, changeType) => | ||
|
@@ -436,6 +444,7 @@ private void UpdateProject(string projectFilePath) | |
UpdateReferences(project, projectFileInfo.ProjectReferences, projectFileInfo.References); | ||
UpdateAnalyzerReferences(project, projectFileInfo); | ||
UpdateAdditionalFiles(project, projectFileInfo.AdditionalFiles); | ||
UpdateAnalyzerConfigFiles(project, projectFileInfo.AnalyzerConfigFiles); | ||
UpdateProjectProperties(project, projectFileInfo); | ||
|
||
_workspace.TryPromoteMiscellaneousDocumentsToProject(project); | ||
|
@@ -483,10 +492,32 @@ private void UpdateAdditionalFiles(Project project, IList<string> additionalFile | |
|
||
foreach (var file in additionalFiles) | ||
{ | ||
if (File.Exists(file)) | ||
{ | ||
if (File.Exists(file)) | ||
{ | ||
_workspace.AddAdditionalDocument(project.Id, file); | ||
} | ||
} | ||
} | ||
} | ||
|
||
private void UpdateAnalyzerConfigFiles(Project project, IList<string> analyzerConfigFiles) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this code be conditioned on editorconfig being enabled in OmniSharp? |
||
{ | ||
if (!_workspace.EditorConfigEnabled) | ||
{ | ||
return; | ||
} | ||
|
||
var currentAnalyzerConfigDocuments = project.AnalyzerConfigDocuments; | ||
JoeRobich marked this conversation as resolved.
Show resolved
Hide resolved
|
||
foreach (var document in currentAnalyzerConfigDocuments) | ||
{ | ||
_workspace.RemoveAnalyzerConfigDocument(document.Id); | ||
} | ||
|
||
foreach (var file in analyzerConfigFiles) | ||
{ | ||
if (File.Exists(file)) | ||
{ | ||
_workspace.AddAnalyzerConfigDocument(project.Id, file); | ||
} | ||
} | ||
} | ||
|
||
|
This file was deleted.
This file was deleted.
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 the same should be applied here
omnisharp-roslyn/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfoExtensions.cs
Line 61 in 3163c23