Do not use gitignore if explicitly passing excludes

[ktbarrett]

Closes #2164.

Changes behavior of how .gitignore is handled. With this change, the rules in .gitignore are only used as a fallback if no exclusion rule is explicitly passed on the command line or in pyproject.toml. Previously they were used regardless if explicit exclusion rules were specified, preventing any overriding of .gitignore rules.

Those that depend only on .gitignore for their exclusion rules will not be affected. Those that use both .gitignore and exclude will find that exclude will act more like actually specifying exclude and not just another extra-excludes. If the previous behavior was desired, they should move their rules from exclude to extra-excludes.

[ktbarrett]

If this is deemed "messy" a superior solution might involve making the exclude parameter a callable object that defaults to a function that checks the gitignore and default rules, or just returns match.search bound method.

[ktbarrett]

Fixed the CHANGE.md issue.

[cooperlees]

LGTM and a good outcome from your issue chat.

Please just make the linter happy.

[ichard26]

While I'm still not 100% happy with the level of disruption this might cause, this does seem like the best solution in terms of costs vs benefit. The semantics should make much more sense after this.

If this is deemed "messy"

Nah, it's fine, I don't like it too much since include used to work the same as exclude and now it doesn't, but the complexity of a callable is not worth it IMO. I'd appreciate a comment describing this behaviour in the code, but other than that, I'll approve this once mine and Cooper's comments have been addressed.

Other than my specific comments, I'd like to see some tests enforcing this functionality / behaviour. I don't want this behaviour to change suddenly. Honestly the fact no test failed from this change makes me feel the existing tests for this logic are not extensive enough.

Thank you!

[ktbarrett]

I find it odd that the primers all passed last night with the same logic. I'll investigate.

[ichard26]

@ktbarrett looks like tox reformatted their code to the newest version of Black today. In that case, just mark tox as a project with no expected changes:

black/src/black_primer/primer.json

Lines 113 to 119 in b39999d

	"tox": {
	"cli_arguments": [],
	"expect_formatting_changes": true,
	"git_clone_url": "https://github.com/tox-dev/tox.git",
	"long_checkout": false,
	"py_versions": ["all"]
	},

edit: yep they did today: tox-dev/tox@a0f0572

[ktbarrett]

The failing test decided to take >1 hour and then fail, while the rest completed in <2 minutes. I'm not entirely sure what that test is doing? Is it just testing running black more than once on a source does the same thing each time? I'm just going to try it again...

[ichard26]

Sounds like either bad luck or a hypothesmith bug, but there's like no way this PR is causing that random failure. Anyway, I'll review this sometime soon, probably tomorrow (it's once again night where I am).

Is it just testing running black more than once on a source does the same thing each time

Well it generates valid code and runs Black over it repeatedly. This is called fuzzing and it's actually a good way of finding edge cases (when it works properly and its randominess doesn't screw itself over)

[ambv]

I'm not a big fan of hiding the default value of --exclude. Now it no longer appears in the tool's --help. See what it did before and re-add it manually to the help text.

[ichard26]

We're getting closer to this becoming ready to merge! Good work! I got some final suggestions / comments but they're nothing major.

p.s. apologies for the wait for my review

[ktbarrett]

Resolved conflicts.

[ktbarrett]

Thanks everyone for the reviews and comments, and for getting this change in.