Adding support for editorconfig #927
Conversation
|
I still need to update the doc, but this is ready for review..... actually looks like there are some test failures. Possibly a linux vs windows path issue. |
|
@shocklateboy92 this was ready for review if you wanted to take a gander. It is a bigger change so I didn't want to skip over your watchful eye. |
|
|
||
| internal static class ConfigFileParser | ||
| { | ||
| private static readonly Regex SectionRegex = new(@"^\s*\[(([^#;]|\\#|\\;)+)\]\s*([#;].*)?$"); |
There was a problem hiding this comment.
So, I'm not a super big fan of parsing source using regex.
People have built libraries that use actual parsers and lexers to handle all the weird corner cases properly.
editorconfig.org recommends:
https://github.com/editorconfig/editorconfig-core-net#readme
However, editorconfig is actually an INI file and there are plenty of well known libraries for those.
There was a problem hiding this comment.
I was originally using that project, but it didn't support IFileSystem - https://github.com/editorconfig/editorconfig-core-net/pulls
I also realized it was built to find a editorconfig for a given file, which would be too slow for what csharpier needs.
That regex comes out of editorconfig-core-net, I did find https://github.com/rickyah/ini-parser just now. I will see if it is easy to pull in.
|
Using ini-parser cleaned up the code and should do a much better job of parsing the files correctly. |
| var sections = new List<Section>(); | ||
| foreach (var section in configData.Sections) | ||
| { | ||
| sections.Add(new Section(section, directory)); |
There was a problem hiding this comment.
nit: I feel like this whole loop could have been a .Select().
| } | ||
|
|
||
| var directoryInfo = fileSystem.DirectoryInfo.FromDirectoryName(directoryName); | ||
| var editorConfigFiles = directoryInfo |
There was a problem hiding this comment.
Thus only a part of the .editorconfig of the project is considered. Since the project system / Visual Studio with <EditorConfigFiles /> allows to load further files. In addition Visual Studios extension the Global AnalyzerConfig.
There was a problem hiding this comment.
This comes from the doc on AnalyzerConfig, so I don't think there need to be any changes to support it.
global configuration files can't be used to configure editor style settings for IDEs, such as indent size or whether to trim trailing whitespace. Instead, they are designed purely for specifying project-level analyzer configuration options.
I wasn't aware of EditorConfigFiles but I'll look into it.
There was a problem hiding this comment.
It sounds like usage of EditorConfigFiles is not recommended and Global AnalyzerConfig should be used instead. https://github.com/MicrosoftDocs/visualstudio-docs/issues/6051#issuecomment-1092262347 Which makes it sounds like this is more intended to be used for configuring analyzers.
There is an easy workaround of just adding an .editorconfig or .csahrpierrc to the root, so it doesn't seem critical to support EditorConfigFiles.
closes #630