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

Fix 4232 #4539

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix 4232 #4539

wants to merge 4 commits into from

Conversation

MeGaGiGaGon
Copy link
Contributor

@MeGaGiGaGon MeGaGiGaGon commented Dec 24, 2024

Description

This fixes #4232. This issue happening is different from what was stated. At least with the given bash script, black does correctly find and apply the settings from the pyproject.toml, the force exclude just doesn't get applied. This happens because the check for if formatting should be skipped when using stdin is constructed via

            root_relative_path = best_effort_relative_path(path, root).as_posix()
            root_relative_path = "/" + root_relative_path

And in the case of the given bash script, root_relative_path ends up as /testfile.py, which obviously doesn't include submodule, so it passes the exclude.

If I understand the intended use case correctly, this is for use with an editor, which is why I put the changelog entry into the integrations section. This should make the behavior of black always match what the docs say,

Tip: if you need Black to treat stdin input as a file passed directly via the CLI, use --stdin-filename. Useful to make sure Black will respect the --force-exclude option on some editors that rely on using stdin.

It also just makes sense to me, even if it's a bit redundant. If you give an input to --stdin-filename that matches the force exclude regex, it should get excluded. If there is something I'm missing with this please let me know.

There might be a better way to do this that doesn't end up in two different calls to path_is_excluded, but it's a relatively cheap function so it should be fine.

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • [N/A (I don't know how you would test this)] Add / update tests if necessary?
  • Add new / update outdated documentation?

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.

when using --stdin-filename , cannot override auto-found project root with --config
1 participant