Open bram-tv opened 9 months ago
Looking at the is_skipped
code in settings.py, it ends with:
https://github.com/PyCQA/isort/blob/7de182933fd50e04a7c47cc8be75a6547754b19c/isort/settings.py#L646
...
# 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 file_path
is a directory it will return False
which will cause it to traverse the directory.
A possible fix: when file_path
is a directory then check if there git_ls_files
contains a file inside that directory.
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:
...
sub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled....
...
which is good.
However it's not yet perfect... When the repository is changed into:
tree -a -I '.git'
.
├── bar.py
├── dir1
│ ├── sub1
│ │ ├── dir1_sub1_file1.py
│ │ └── dir1_sub1_file2.py
│ ├── sub2
│ │ ├── dir1_sub2_file1.py
│ │ └── dir1_sub2_file2.py
│ └── sub3
│ ├── dir1_sub3_file1.py
│ └── dir1_sub3_file2.py
├── foo.py
└── .gitignore
4 directories, 9 files
where the .gitignore file now contains:
dir1/sub1/
dir1/sub2/
dir1/sub3/
then the output is:
...
dir1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
...
which is a bit confusing: in the .gitignore file it's specified to ignore dir1/sub1/, dir1/sub2 and dir1/sub3 but the isort
output makes it appear as if the entire directory was ignored..
In a way it was because there were no other files in dir1/ but still the message doesn't "feel" right..
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:
$ tree -a -I '.git'
.
├── bar.py
├── dir1
│ ├── sub1
│ │ ├── dir1_sub1_file1.py
│ │ ├── dir1_sub1_file2.py
│ │ ├── subsub1
│ │ │ └── dir1_sub1_subsub1_file_1.py
│ │ ├── subsub2
│ │ │ └── dir1_sub1_subsub2_file_1.py
│ │ ├── subsub3
│ │ │ └── dir1_sub1_subsub3_file_1.py
│ │ ├── subsub4
│ │ │ └── dir1_sub1_subsub4_file_1.py
│ │ ├── subsub5
│ │ │ └── dir1_sub1_subsub5_file_1.py
│ │ ├── subsub6
│ │ │ └── dir1_sub1_subsub6_file_1.py
│ │ ├── subsub7
│ │ │ └── dir1_sub1_subsub7_file_1.py
│ │ ├── subsub8
│ │ │ └── dir1_sub1_subsub8_file_1.py
│ │ └── subsub9
│ │ └── dir1_sub1_subsub9_file_1.py
│ ├── sub2
│ │ ├── dir1_sub2_file1.py
│ │ └── dir1_sub2_file2.py
│ └── sub3
│ ├── dir1_sub3_file1.py
│ └── dir1_sub3_file2.py
├── foo.py
└── .gitignore
13 directories, 18 files
And assume the file dir1/sub1/subsub5/dir1_sub1_subsub5_file_1.py was committed (despite being ignored, i.e. it was added using git add -f
)
And .gitignore contains:
dir1/sub1/
dir1/sub2/
dir1/sub3/
The git ls-files
output for reference:
$ git ls-files
.gitignore
bar.py
dir1/sub1/subsub5/dir1_sub1_subsub5_file_1.py
foo.py
$ git ls-files --others --exclude-standard
$
then isort
output is:
...
sub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub7 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub4 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub9 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub6 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub8 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub1_file2.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub1_file1.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
...
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..
We know the files that may need to be checked (i.e. the files in self.git_ls_files
) but that's not how the code loops..
Preamble
There are several issues about .gitignore that looks similar but none are quite the same.. The one that comes closest is https://github.com/PyCQA/isort/issues/1912#issuecomment-1124738985 and more exact the comment added in https://github.com/PyCQA/isort/pull/1900#issuecomment-1072709473 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)