From 0eb9c068bf6164c3fd0fbb51c4d4706f26d70384 Mon Sep 17 00:00:00 2001 From: Bela VanderVoort Date: Thu, 23 Nov 2023 13:59:53 -0600 Subject: [PATCH] Optimize editorconfig lookups when piping files (#1039) * Optimize editorconfig lookups when piping files This is gonna cause conflicts with #1030 * self code review --- Src/CSharpier.Cli/CommandLineFormatter.cs | 3 +- .../EditorConfig/EditorConfigParser.cs | 12 +++-- Src/CSharpier.Cli/Options/OptionsProvider.cs | 4 +- Src/CSharpier.Cli/Program.cs | 1 - Src/CSharpier.Tests/OptionsProviderTests.cs | 48 ++++++++++++++++++- 5 files changed, 60 insertions(+), 8 deletions(-) diff --git a/Src/CSharpier.Cli/CommandLineFormatter.cs b/Src/CSharpier.Cli/CommandLineFormatter.cs index cd74f9e1d..c2715156d 100644 --- a/Src/CSharpier.Cli/CommandLineFormatter.cs +++ b/Src/CSharpier.Cli/CommandLineFormatter.cs @@ -44,7 +44,8 @@ CancellationToken cancellationToken commandLineOptions.ConfigPath, fileSystem, logger, - cancellationToken + cancellationToken, + limitEditorConfigSearch: true ); if ( diff --git a/Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs b/Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs index bac4df510..59c98b0c3 100644 --- a/Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs +++ b/Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs @@ -6,13 +6,15 @@ namespace CSharpier.Cli.EditorConfig; // keep track of directories + their corresponding configs // if searching a new directory and we go up to a parent that contains the first config, use that // how many directories would we run into? enough to matter? -// TODO !!!!!!!!!!!!!!!!!! an easy fix, is to pass a flag to not go deeper when running this via piping files!!! internal static class EditorConfigParser { /// Finds all configs above the given directory as well as within the subtree of this directory public static List FindForDirectoryName( string directoryName, IFileSystem fileSystem, + // not the best name, but I plan on rewriting this to not find all of the configs up front + // which will remove this parameter + bool limitEditorConfigSearch, IgnoreFile ignoreFile ) { @@ -22,9 +24,13 @@ IgnoreFile ignoreFile } var directoryInfo = fileSystem.DirectoryInfo.FromDirectoryName(directoryName); - // TODO this is probably killing performance if nothing else when piping a single file var editorConfigFiles = directoryInfo - .EnumerateFiles(".editorconfig", SearchOption.AllDirectories) + .EnumerateFiles( + ".editorconfig", + limitEditorConfigSearch + ? SearchOption.TopDirectoryOnly + : SearchOption.AllDirectories + ) .Where(x => !ignoreFile.IsIgnored(x.FullName)) .ToList(); diff --git a/Src/CSharpier.Cli/Options/OptionsProvider.cs b/Src/CSharpier.Cli/Options/OptionsProvider.cs index fa07e8b12..489c04e8a 100644 --- a/Src/CSharpier.Cli/Options/OptionsProvider.cs +++ b/Src/CSharpier.Cli/Options/OptionsProvider.cs @@ -35,7 +35,8 @@ public static async Task Create( string? configPath, IFileSystem fileSystem, ILogger logger, - CancellationToken cancellationToken + CancellationToken cancellationToken, + bool limitEditorConfigSearch = false ) { var specifiedPrinterOptions = configPath is not null @@ -55,6 +56,7 @@ CancellationToken cancellationToken editorConfigSections = EditorConfigParser.FindForDirectoryName( directoryName, fileSystem, + limitEditorConfigSearch, ignoreFile ); } diff --git a/Src/CSharpier.Cli/Program.cs b/Src/CSharpier.Cli/Program.cs index f4aa95653..d9e87d478 100644 --- a/Src/CSharpier.Cli/Program.cs +++ b/Src/CSharpier.Cli/Program.cs @@ -1,6 +1,5 @@ using System.CommandLine; using System.CommandLine.Invocation; -using System.Diagnostics; using System.IO.Abstractions; using System.Text; using Microsoft.Extensions.Logging; diff --git a/Src/CSharpier.Tests/OptionsProviderTests.cs b/Src/CSharpier.Tests/OptionsProviderTests.cs index be3adaee8..1215dd79c 100644 --- a/Src/CSharpier.Tests/OptionsProviderTests.cs +++ b/Src/CSharpier.Tests/OptionsProviderTests.cs @@ -587,6 +587,48 @@ public async Task Should_Prefer_Closer_CSharpierrc() result.TabWidth.Should().Be(1); } + [Test] + public async Task Should_Not_Look_For_Subfolders_EditorConfig_When_Limited() + { + var context = new TestContext(); + context.WhenAFileExists( + "c:/test/subfolder/.editorconfig", + @" + [*] + indent_size = 1 + " + ); + + // this shouldn't happen in the real world, but validates we correctly limit + // the search to the top directory only + var result = await context.CreateProviderAndGetOptionsFor( + "c:/test/", + "c:/test/subfolder/test.cs", + limitEditorConfigSearch: true + ); + result.TabWidth.Should().Be(4); + } + + [Test] + public async Task Should_Look_For_Subfolders_When_Limited() + { + var context = new TestContext(); + context.WhenAFileExists( + "c:/test/.editorconfig", + @" + [*] + indent_size = 1 + " + ); + + var result = await context.CreateProviderAndGetOptionsFor( + "c:/test/subfolder", + "c:/test/subfolder/test.cs", + limitEditorConfigSearch: true + ); + result.TabWidth.Should().Be(1); + } + private static void ShouldHaveDefaultOptions(PrinterOptions printerOptions) { printerOptions.Width.Should().Be(100); @@ -611,7 +653,8 @@ public void WhenAFileExists(string path, string contents) public async Task CreateProviderAndGetOptionsFor( string directoryName, - string filePath + string filePath, + bool limitEditorConfigSearch = false ) { if (!OperatingSystem.IsWindows()) @@ -626,7 +669,8 @@ string filePath null, this.fileSystem, NullLogger.Instance, - CancellationToken.None + CancellationToken.None, + limitEditorConfigSearch ); return provider.GetPrinterOptionsFor(filePath);