Skip to content

Remove references to defunct git_ignore config#2531

Merged
DanielNoord merged 1 commit intoPyCQA:mainfrom
sparrowt:remove-defunct-option
May 7, 2026
Merged

Remove references to defunct git_ignore config#2531
DanielNoord merged 1 commit intoPyCQA:mainfrom
sparrowt:remove-defunct-option

Conversation

@sparrowt
Copy link
Copy Markdown
Contributor

@sparrowt sparrowt commented May 7, 2026

https://pycqa.github.io/isort/docs/configuration/options.html#git-ignore is nonsense and doesn't work - it raises UnsupportedSettings because e168069 removed it without removing the documentation that refers to it.

The remaining config option that still works is skip_gitignore https://pycqa.github.io/isort/docs/configuration/options.html#skip-gitignore.

Remove the defunct one to avoid confusion.

Fixes #1920

It raises `UnsupportedSettings` because e168069
removed it without removing the documentation that refers to it.

The remaining config option that still works is `skip_gitignore`.
@Foryah
Copy link
Copy Markdown

Foryah commented May 7, 2026

It's true that git_ignore was removed but I think it's more accurate to say it was replaced with git_ls_files as we can see in the commit. This PR solves half the problem, to solve #1920 we still need to document how git_ls_files works 😁

@sparrowt
Copy link
Copy Markdown
Contributor Author

sparrowt commented May 7, 2026

I think the real question is whether git_ls_files should really be in _Config at all - AFAICS it's more of an implementation detail, caching the result of calling git ls-files (which is likely to be pretty big for most repos) and something which I can't imagine anyone wanting to pass in as a config.

So I think this PR probably does all of the documentation changes that we want related to this - perhaps a separate ticket is warranted for whether git_ls_files should be moved out of the config class seeing as that doesn't quite seem appropriate, but I'm new to this codebase so may have missed something & didn't want to go trying to move functional code around, to keep this "avoid confusion" PR simple.

@Foryah
Copy link
Copy Markdown

Foryah commented May 7, 2026

Makes a lot of sense 😁 I never read the code other than the commit you passed and there I just saw that it was a replacement so I assumed is similar funtionality, different flag. But yeah, if the functionality is different, then all good 🥳

Copy link
Copy Markdown
Member

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Thanks!

@DanielNoord DanielNoord enabled auto-merge May 7, 2026 20:40
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.15%. Comparing base (ab9cd36) to head (a53fecb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2531   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files          41       41           
  Lines        3090     3090           
  Branches      668      668           
=======================================
  Hits         3064     3064           
  Misses         14       14           
  Partials       12       12           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@DanielNoord DanielNoord added this pull request to the merge queue May 7, 2026
Merged via the queue into PyCQA:main with commit 5e24c24 May 7, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

git_ignore documentation could be improved

3 participants