Skip to content
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

Allow EditorConfigFile to be used in-memory without access or use of physical file system. #25

Merged
merged 3 commits into from
Nov 24, 2023

Conversation

maxkatz6
Copy link
Contributor

@maxkatz6 maxkatz6 commented Nov 8, 2023

Hi @Mpdreamz !

I was going to use EditorConfig in Avalonia XAML compiler, and found couple of blocking problems that I think can be easily avoidable with this PR:

  1. Currently EditorConfigParser forces to use editorconfig files from the file system defined in the same folder and up. It is technically a standard behavior, but MSBuild/VisualStudio can create temporal editorconfig files in an intermediate (obj) folder. Our compiler receives an array of file paths as input from the build system, and we should be able to parse them instead of iterating manually.
  2. Some APIs accept EditorConfigFile as an input, but it's not possible to actually use them, since constructors were internal. This PRs adds some factory methods (two - one with a file path and the second with a TextReader parameter).
  3. This library assumes that developers always have access to the file system, and we can synchronously read from it. It might not be the case in sandboxed environments. Note: it's not really the problem for Avalonia, but I can imagine editorconfig to be usable in online editors in the browser-wasm. This problem is solved by providing Parse(TextReader) overload, which can be used from the in-memory file contents.
  4. EditorConfigFile instances can be reused and cached, but this problem was already solved in previous PRs in another way. So it's not really critical.

select section;

return new FileConfiguration(ParseVersion, file, sections.ToList());
}

private bool IsMatch(string glob, string fileName, string directory)
private bool IsMatch(string glob, string fileName)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"directory" parameter wasn't used here. So I removed it just to be sure that EditorConfig without a directory can be used as well. I.e. matching section glob directly to the file name.

@Mpdreamz
Copy link
Member

Mpdreamz commented Nov 8, 2023

This looks great!

I'm currently on PTO but will look into pulling this in asap early next week!

Copy link
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! only had to update the test to be a tad more x-plat friendly.

@Mpdreamz Mpdreamz merged commit 295f0af into editorconfig:master Nov 24, 2023
3 checks passed
@Mpdreamz
Copy link
Member

This should now be up on NuGet in version 0.15.0 (despite the failed Github Action 😢).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants