Skip to content
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

Bugfix black .gitignore usage #515

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Conversation

jedie
Copy link
Contributor

@jedie jedie commented Aug 9, 2023

Black didn't use the .gitignore file, because gen_python_files() will be called with gitignore_dict=None and None will disable the .gitignore usage.

see: #323 (comment)

Fixes #490.

Copy link
Collaborator

@MatthijsBurgh MatthijsBurgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akaihola
Copy link
Owner

akaihola commented Feb 23, 2024

This is great, thanks @jedie and @MatthijsBurgh.

Three questions come to mind:

  • Might this change break something if using an old version of Black? If so, the minimum required Black version might need to be bumped. Which Black version does psf/black@35ce47e affect?
  • How to write a unit or integration test for this?
  • Why are all GitHub checks "Expected - Waiting for status to be reported" after @jedie submitted the patch?

@akaihola akaihola marked this pull request as draft February 23, 2024 10:22
@akaihola akaihola added the bug Something isn't working label Feb 23, 2024
@akaihola akaihola added this to the Darker 1.8.1 milestone Feb 23, 2024
@MatthijsBurgh
Copy link
Collaborator

I checked the blame, I think the the minimal required version of black is 21.5b1. Currently it is on 21.7b1, so fine.

@akaihola akaihola force-pushed the jedie-patch-1 branch 3 times, most recently from 564c100 to 15da388 Compare February 26, 2024 22:01
@akaihola akaihola marked this pull request as ready for review February 26, 2024 22:05
@akaihola
Copy link
Owner

I wrote three unit tests, matching the three different versions of gen_python_files in supported Black versions.

Actually {} is a valid value only for the most modern versions which use the gitignore_dict argument. The older versions with a gitignore argument must still get None as the value instead. I fixed that too.

@akaihola akaihola self-assigned this Feb 26, 2024
@akaihola akaihola modified the milestones: Darker 1.8.1, Darker 1.7.3 Feb 26, 2024
@akaihola
Copy link
Owner

It's ok for the future test to fail. Updating Black to 24.2 and beyond is now prevented for Darker 1.7.x, and version 1.8.0 will fix this compatibility issue and allow newer Black versions again.

@akaihola akaihola merged commit 277b6b3 into akaihola:master Feb 26, 2024
20 of 21 checks passed
@jedie jedie deleted the jedie-patch-1 branch February 27, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

Git subprocess call error when too many files in directory, even if gitignore'd
3 participants