-
-
Notifications
You must be signed in to change notification settings - Fork 578
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
--skip-gitignore
still traverses files/directories in .gitignore'd directories
#2237
Comments
Looking at the Line 646 in 7de1829
...
# git_ls_files are good files you should parse. If you're not in the allow list, skip.
if (
git_folder
and not file_path.is_dir()
and str(file_path.resolve()) not in self.git_ls_files[git_folder]
):
return True
return False When the A possible fix: when Something like: diff --git a/isort/settings.py b/isort/settings.py
index a3658fff..67acdbf5 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -641,11 +641,14 @@ class Config(_Config):
# git_ls_files are good files you should parse. If you're not in the allow list, skip.
- if (
- git_folder
- and not file_path.is_dir()
- and str(file_path.resolve()) not in self.git_ls_files[git_folder]
- ):
+ if git_folder:
+ if file_path.is_dir():
+ dir = file_path.resolve()
+ if not any(file.startswith(f"{dir}/") for file in self.git_ls_files[git_folder]):
+ return True
+
+ elif str(file_path.resolve()) not in self.git_ls_files[git_folder]:
+ return True
return True
return False With the example repository listed in the description the output is:
which is good. However it's not yet perfect...
where the .gitignore file now contains:
then the output is:
which is a bit confusing: in the .gitignore file it's specified to ignore dir1/sub1/, dir1/sub2 and dir1/sub3 but the |
Also: note that this solution is still suboptimal is some cases: it can still cause too many files/directories to be traversed. Assume the layout:
And assume the file dir1/sub1/subsub5/dir1_sub1_subsub5_file_1.py was committed (despite being ignored, i.e. it was added using And .gitignore contains:
The
then
so it still did some unneeded directory traversal. I'm not sure if the structure of the code allows this to be (easily) fixed tho.. |
Preamble
There are several issues about .gitignore that looks similar but none are quite the same..
The one that comes closest is #1912 (comment) and more exact the comment added in #1900 (comment) but it doesn't look like a specific issues was created for it. (#1912 is still open tho but the title does not really match with this issue).
Example
Let's start with an example repository:
The contents of dir1/.gitignore:
Just for reference: the output of
git ls-files
Running
isort --skip-gitignore --verbose .
:It did skip the files inside dir1/sub1, dir1/sub2/ and dir1/sub3/ which is good but it really shouldn't be aware that these file exist. It means it did traverse into the sub-directories and then listed the files in it..
Work-around
Using
--extend-skip-glob
as a work-around and specifying the contents of the .gitignore file:Now it skipped the sub directories entirely and didn't traverse inside the directories to list the files (to then ignore these)
The text was updated successfully, but these errors were encountered: