Closed jamesfoster closed 1 month ago
[EDIT]
Finds all configs above the given directory as well as within the subtree of this directory
Yikes. What if I run this at /
? You're going to scan my whole hard-drive? No thank you.
1) When reading from stdin, you should only use config files from pwd
and above. You could even use the one in my home directory if no other is found? But definitely don't do a recursive scan.
1) When formatting a named file, same as above. You know the file to format. Only look in the same directory as the file and above. No need to do a recursive scan.
1) When formatting all files in a directory:
* when you do a recursive directory scan (whether to find config files or code files) you should handle access denied exceptions. You probably want to print to stderr noting the denied access but continuing (or continue silently).
* Maybe only load config files when you find a code file to format, that way you never need to recursively scan the subtree for config files at any point, you just need to walk back up the directory tree. You can cache the config files you find as you go.
https://learn.microsoft.com/en-us/dotnet/api/system.io.searchoption?view=net-8.0
An easy way to get csharpier to not go poking around is to supply a path to the config file with --config-path
. It does require the file to exist, but doesn't mind too much if the file is empty.
I had created #288 to allow you to specify --stdin-filepath
similiar to prettier. That would allow you to pipe to csharpier from anywhere but limit the scope of where it looks for config files. Would have been another work around to keep csharpier from poking around.
Thanks. If I use this in a script I'll add --config-path
for performance. However, it's not very ergonomic to have to do that in the terminal when I just want to pipe a file to it. Same with https://github.com/belav/csharpier/issues/288.
https://github.com/belav/csharpier/issues/1228 does sound like the way to go. My gut feel is that directory scanning has to be slower. Do you have any benchmarks around the config scanning code? I could look at implementing this if you're open to contributions?
It turns out that when I added the code to search subtrees ahead of time for all config files I limited the editorconfig searching when std in was piped in, but neglected to also limit the standard config searching. #1243 is a quick fix for the problem you have.
I don't know that I've benchmarked this specifc problem, but at one point each file that was formatted was redoing the scanning + parsing for the csharpier ignore file. I think it was the reparsing of that file that was the main performance problem.
Definitely open to contributions if you feel like taking it on, but #1228 seems less important with this quick fix.
@belav BTW, I had a look a few weeks back to implement this. However, a significant portion of the tests fail on master
. Mostly due to line ending differences. I meant to mention it at the time but stuff happens and it slipped my mind.
Few solutions.
1) use .gitattributes
with eol=lf
to enforce commit & checkout to normalize line endings.
2) In tests, always compare strings with normalized line endings. (e.g. using string.ReplaceLineEndings()
)
Thoughts?
Environments:
dotnet csharpier
Steps to reproduce: run csharpier from the command line in a folder with folders you don't have access to
Expected behavior: It should write the formatted text to stdout and NOT go poking about in directories you have no business accessing!!
Actual behavior: