Skip to content

Commit

Permalink
Optimize editorconfig lookups when piping files (#1039)
Browse files Browse the repository at this point in the history
* Optimize editorconfig lookups when piping files

This is gonna cause conflicts with #1030

* self code review
  • Loading branch information
belav authored Nov 23, 2023
1 parent 82dadf4 commit 0eb9c06
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 8 deletions.
3 changes: 2 additions & 1 deletion Src/CSharpier.Cli/CommandLineFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ CancellationToken cancellationToken
commandLineOptions.ConfigPath,
fileSystem,
logger,
cancellationToken
cancellationToken,
limitEditorConfigSearch: true
);

if (
Expand Down
12 changes: 9 additions & 3 deletions Src/CSharpier.Cli/EditorConfig/EditorConfigParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
/// <summary>Finds all configs above the given directory as well as within the subtree of this directory</summary>
public static List<EditorConfigSections> 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
)
{
Expand All @@ -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();

Expand Down
4 changes: 3 additions & 1 deletion Src/CSharpier.Cli/Options/OptionsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public static async Task<OptionsProvider> Create(
string? configPath,
IFileSystem fileSystem,
ILogger logger,
CancellationToken cancellationToken
CancellationToken cancellationToken,
bool limitEditorConfigSearch = false
)
{
var specifiedPrinterOptions = configPath is not null
Expand All @@ -55,6 +56,7 @@ CancellationToken cancellationToken
editorConfigSections = EditorConfigParser.FindForDirectoryName(
directoryName,
fileSystem,
limitEditorConfigSearch,
ignoreFile
);
}
Expand Down
1 change: 0 additions & 1 deletion Src/CSharpier.Cli/Program.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
48 changes: 46 additions & 2 deletions Src/CSharpier.Tests/OptionsProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -611,7 +653,8 @@ public void WhenAFileExists(string path, string contents)

public async Task<PrinterOptions> CreateProviderAndGetOptionsFor(
string directoryName,
string filePath
string filePath,
bool limitEditorConfigSearch = false
)
{
if (!OperatingSystem.IsWindows())
Expand All @@ -626,7 +669,8 @@ string filePath
null,
this.fileSystem,
NullLogger.Instance,
CancellationToken.None
CancellationToken.None,
limitEditorConfigSearch
);

return provider.GetPrinterOptionsFor(filePath);
Expand Down

0 comments on commit 0eb9c06

Please sign in to comment.