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

Respect isort's skip_glob setting #751

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Respect isort's skip_glob setting #751

wants to merge 6 commits into from

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Oct 2, 2024

Fixes #615.

@akaihola akaihola added the bug Something isn't working label Oct 2, 2024
@akaihola akaihola added this to the Darker 3.0.1 milestone Oct 2, 2024
@akaihola akaihola self-assigned this Oct 2, 2024
@akaihola akaihola marked this pull request as draft October 2, 2024 14:53
@akaihola
Copy link
Owner Author

akaihola commented Oct 2, 2024

@ranelpadon I started by successfully adding a failing unit test. Next, looking deeper into the reason for the wrong behavior. It's somehow related to the fact that Darker calls isort's API directly instead of running it as a subprocess.

I invited you as a collaborator on this repository, hoping you could be the reviewer for this PR. Would you have time for that?

@akaihola
Copy link
Owner Author

akaihola commented Oct 2, 2024

Ok @ranelpadon there seem to be two reasons for isort ignoring skip_glob =:

  1. a bug in passing the directory of the configuration file to isort
  2. we pass Python code directly to isort.code instead of letting it read the file from the disk (see darker.import_sorting:137), and in this case isort simply disregards the skip_glob = setting (see isort.api:188)

Reason 1. is easily fixed, but 2. is a trickier one. I'll need to spend a little more time on it.

@akaihola
Copy link
Owner Author

akaihola commented Oct 2, 2024

Not so difficult to fix after all. @ranelpadon could you verify that this branch works for you?

This PR is best reviewed one commit at a time.

@akaihola akaihola marked this pull request as ready for review October 2, 2024 17:15
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.

isort: skip_glob option doesn't work
1 participant