Added logging for .editorconfig paths #2018
Conversation
| @@ -533,6 +533,7 @@ private void UpdateAnalyzerConfigFiles(Project project, IList<string> analyzerCo | |||
| { | |||
| if (!_workspace.EditorConfigEnabled) | |||
| { | |||
| _logger.LogDebug($".editorconfig files were configured by the project {project.Name} but will not be respected because the feature is switched off in OmniSharp. Enable .editorconfig support in OmniSharp to take advantage of them."); | |||
There was a problem hiding this comment.
IMO this should be Warning or at least Information.
There was a problem hiding this comment.
I am not sure. Editorconfig is off by default in OmniSharp, and I don't think we should flood with warnings when our own feature is opt-in.
There was a problem hiding this comment.
I'd agree if it was the other way round though
There was a problem hiding this comment.
According to log message above, it seems like we know that .editoconfig is configured by the project (do we also know if there are .editorconfig files found?). If that's the case, then I think we should inform the user that in order to take advantage of a feature configured by the project, then the user should also enable it in OmniSharp. In that case my opinion is that it's at least Information level.
But if this is a log message seen for every project, regardless if .editorconfig is intended to be used or not. Then go with debug. But maybe rephrase the log message?
There was a problem hiding this comment.
yes we know that it's configured on the project but that is still too invasive for me. update project runs many times during your work on a project so you shouldn't see the warning all the time.
If you want to have a user information mechanism then this logging is not the mechanism to do it, it's strictly for debugging purposes. The phrasing is such to aid self-debugging too.
for what you are suggesting we should just have a regular one-time message in the logs that says - editorconfig is currently disabled that is always visible.
There was a problem hiding this comment.
You are correct, because this message is shown on every Project update, it's too invasive. Didn't notice that during first review.
for what you are suggesting we should just have a regular one-time message in the logs that says - editorconfig is currently disabled that is always visible.
Yes, this is what I was looking for initially. A single warning (or information) if a loaded project has .editorconfig configured, but support is disabled in OmniSharp.
|
This is a great response to my issue - thank you! <3 |
|
Anything I can do to help with the failing build? Looking forward to this being merged in and released! |
|
it's a flaky build - unrelated to the change. |
|
Ah I see. So in about a week or so? |
|
this is superseded by #2028 |
|
@filipw now that that PR has been shipped, when can we expect this to arrive in the Visual Studio Code Omnisharp extension? |
|
we ship OmniSharp server about once a month https://github.com/OmniSharp/omnisharp-roslyn/blob/master/CHANGELOG.md#1375---not-yet-released so before Christmas, I think. However, a prerelease is out on each merge. on the VS Code C# extension side, @JoeRobich has the reigns on the release cadence |
related to #2017