Fix: black only respects the root gitignore.

[hadialqattan]

Black now respects .gitignore files in all levels, not only root/.gitignore file (apply .gitignore rules like git does).

Fix: #1730

[hadialqattan]

I noticed that we've got a failure on the test cases out of this PR scope:

=================================== ERRORS ====================================
____________________ ERROR collecting tests/test_blackd.py ____________________
tests\test_blackd.py:10: in <module>
    import blackd
src\blackd\__init__.py:28: in <module>
    _stop_signal = asyncio.Event()
c:\hostedtoolcache\windows\python\3.8.9\x64\lib\asyncio\locks.py:260: in __init__
    self._loop = events.get_event_loop()
c:\hostedtoolcache\windows\python\3.8.9\x64\lib\asyncio\events.py:639: in get_event_loop
    raise RuntimeError('There is no current event loop in thread %r.'
E   RuntimeError: There is no current event loop in thread 'Dummy-1'.
____________________________ ERROR collecting gw0 _____________________________
Different tests were collected between gw1 and gw0. The difference is:
--- gw1

+++ gw0

@@ -101,19 +101,6 @@

For full log: check link

[ichard26]

That's the price we pay for using pytest-xdist, sometimes we hit this bug: pytest-dev/pytest-xdist#620.

I thought it would be uncommon enough to be worth the speed improvements, but I'm start to reconsider. In the meanwhile, I'll rerun the test workflow.

[ichard26]

This is way easier to implement than I had thought! Heh.

There's one case that this patch fails to handle correctly. It's when a parent directory doesn't have a gitignore but the child directory does. Git respects the child's gitignore but this implementation does not :(

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ tree -a --matchdirs -I .git
.
├── a.py
└── some-dir
    ├── b.py
    └── .gitignore

1 directory, 3 files

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ cat some-dir/.gitignore 
*.py

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ git clean -xfn
Would remove some-dir/b.py  # <-- git respects the child's gitignore

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ black . --check -v
.git ignored: matches the --exclude regular expression
a.py already well formatted, good job.
some-dir/b.py already well formatted, good job.  # < -- but not Black :/
All done! ✨ 🍰 ✨
2 files would be left unchanged.

This shouldn't be hard to fix but it will make the patch not as amazingly simple haha :)

This will also require changes to the .gitignore documentation we currently have. If you don't want to handle this part, I wouldn't mind writing the docs!

I'm also starting to really dislike the file collection / discovery tests we have. They need a fair bit of boilerplate and are just annoying to read. This is obviously out of scope for this PR, but I wanted to get it out there.

Thank you for sticking through with this feature! I'm looking forward to when this is merged.

[hadialqattan]

This is way easier to implement than I had thought! Heh.

There's one case that this patch fails to handle correctly. It's when a parent directory doesn't have a gitignore but the child directory does. Git respects the child's gitignore but this implementation does not :(

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ tree -a --matchdirs -I .git
.
├── a.py
└── some-dir
    ├── b.py
    └── .gitignore

1 directory, 3 files

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ cat some-dir/.gitignore 
*.py

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ git clean -xfn
Would remove some-dir/b.py  # <-- git respects the child's gitignore

~/programming/testing/gitignore on master via v3.8.5 (black) 
❯ black . --check -v
.git ignored: matches the --exclude regular expression
a.py already well formatted, good job.
some-dir/b.py already well formatted, good job.  # < -- but not Black :/
All done! ✨ 🍰 ✨
2 files would be left unchanged.

This shouldn't be hard to fix but it will make the patch not as amazingly simple haha :)

This will also require changes to the .gitignore documentation we currently have. If you don't want to handle this part, I wouldn't mind writing the docs!

I'm also starting to really dislike the file collection / discovery tests we have. They need a fair bit of boilerplate and are just annoying to read. This is obviously out of scope for this PR, but I wanted to get it out there.

Thank you for sticking through with this feature! I'm looking forward to when this is merged.

Before fixing the bug:

gitignore + get_gitignore(child) if gitignore else None,

After:

gitignore + get_gitignore(child) if gitignore is not None else None,

yeah, it won't be as simple as the previous patch, lol!

no problem, I updated the docs!

Thanks for the review!

[hadialqattan]

+ I've allowed edits by maintainers so you can improve the testing data/methods before merging if you don't mind!

Have a pythonic day! ❤️

[ichard26]

Pushed an explanatory comment since the if expression made no sense to me until I manually checked the value of gitignore, but other than that LGTM. Thank you very much!

Have a pythonic day! ❤️

You too! 🐍

[hadialqattan]

Nice, but before merging, I'm gonna modify the related tests!

[JelleZijlstra]

Looks good, @ichard26 any reason not to merge yet?

Not sure though why Pipfile.lock changed.

[ichard26]

I just looked into it and the changes in Pipfile.lock are whitespace only. The changes turned the JSON's file indentation from 4 to 2. Effectively this happened: json.dumps(json.loads(old_pipfile_lock), indent=2) + "\n"

I'll undo those changes and also fix the merge conflict and then I'll merge.

ichard26 any reason not to merge yet?

Other than the above, no.