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

Add a include parameter to allow expanding scope of included files #3410

Closed
neingeist opened this issue Mar 8, 2023 · 22 comments · Fixed by #3914
Closed

Add a include parameter to allow expanding scope of included files #3410

neingeist opened this issue Mar 8, 2023 · 22 comments · Fixed by #3914
Assignees
Labels
cli Related to the command-line interface configuration Related to settings and configuration

Comments

@neingeist
Copy link

Ruff 0.0.254 seems to ignore Python scripts that do not end in .py. Is there an option to include them in the configuration?

@charliermarsh
Copy link
Member

We typically require that those files are passed-in explicitly -- e.g., if you do ruff /path/to/file, we'll always check file regardless of the extension; but if you do ruff path/to, we only check .py and .pyi files.

I think this is consistent with Black's behavior (running black foo won't reformat files in foo that don't end in .py).

@charliermarsh charliermarsh added the question Asking for support or clarification label Mar 8, 2023
@neingeist
Copy link
Author

neingeist commented Mar 8, 2023

I see! I absolutely agree that the default behavior is good. However, Black can be configured to include these extra files:

[tool.black]
include = '.*/(bad-movies|test|.*\.py)$'

Something like that I would wish for :) (Black will check bad-movies and test, too, with this configuration.)

@charliermarsh
Copy link
Member

Oh interesting, so that's the file list that's used if you execute black (instead of black /path/to/folder)?

@neingeist
Copy link
Author

Ruff is a great tool, by the way! Need to remind myself to also give positive feedback, not only report bugs/wish for features 😇

@charliermarsh
Copy link
Member

Or does include also apply if you do black /path/to/folder?

@charliermarsh
Copy link
Member

Oh heh thank you!

@charliermarsh
Copy link
Member

I can also just look at the Black docs to answer those questions. I agree it could make sense to support something like that.

@neingeist
Copy link
Author

black without an option doesn't do anything, so it's like ruff. This include is used when I run black . (or black /path/to/src/). It seems to search the path and match against the regex in include.

@charliermarsh
Copy link
Member

Ahhh I see, ok. Lemme think on that. (So our include is roughly *.pyi? right now.)

@neingeist
Copy link
Author

Black's default seems to be include = '\.pyi?$'. While I checked Black's behavior I noticed that it still ignores files in .gitignore.

@charliermarsh charliermarsh changed the title Option to include files? Add a include parameter to allow expanding scope of included files Mar 8, 2023
@charliermarsh charliermarsh added configuration Related to settings and configuration cli Related to the command-line interface and removed question Asking for support or clarification labels Mar 8, 2023
@charliermarsh
Copy link
Member

(We also ignore files in .gitignore.)

@neingeist
Copy link
Author

Ah, I just noticed your example was a glob pattern, mine was a regex (because Black uses a regex here). Probably something to think about a bit.

@ahal
Copy link

ahal commented Mar 10, 2023

This feature would be appreciated! We have some custom file extensions (which are a subset of Python) in a large monorepo. In order to pass them into ruff explicitly, we have a Python wrapper to find them all.. But finding the files takes much longer than it takes ruff to lint the entire monorepo :p.

@charliermarsh
Copy link
Member

😅 😅 😅 Will try to get to this soon.

@charliermarsh charliermarsh self-assigned this Mar 11, 2023
@neingeist
Copy link
Author

neingeist commented Mar 15, 2023

(We also ignore files in .gitignore.)

Yeah, I just wanted to mention it, because Black's behavior seems to be:

  • search the path
  • process files matching include regex (with a sensible default)
  • BUT not ignored files (e.g. .gitignore)

@steve-mavens
Copy link

steve-mavens commented Mar 25, 2023

In view of #2192, would it be useful for this feature to distinguish between, "I want you to check all files matching this pattern" and "I'm throwing the kitchen sink at you here, and I only want you to check all files matching this pattern which also have a #! python"?

Then (assuming a regex-y syntax) someone might say:

include = .*\.pyi?
include-scripts = .*

Which means, "check all .py files, all .pyi files, and all files that contain shebangs"

Or:

include = *.pyi?
include-scripts = ^[^\.]*$

which means, "check all .py files, all .pyi files, and all files with no file extension that contain shebangs".

@charliermarsh
Copy link
Member

The first version of this will go out in the next release -- e.g., you can do:

[tool.ruff]
extend-include = ["*.pyw"]

...to include .pyw files alongside the default .py and .pyi files.

@tylerlaprade
Copy link
Contributor

tylerlaprade commented Aug 23, 2023

Oh, this is incredible!

By the way, my usecase is that I only want to run Ruff on manual Django migrations, not auto-generated ones. We have a convention that they are always named ####_manual_foo, so I tried to match with extend-exclude = ["src/migrations/[0-9][0-9][0-9][0-9]_[!m][!a][!n][!u][!a][!l]*", "typings"], but today we had a confusing hit on 0212_contract_budget....py. Then we realized that the 3rd letter of both is n, so this pattern is incorrect. What I really want is to exclude everything in migrations/, but then to include anything starting with manual, which I'm hoping extend-include will allow me to do.

EDIT: Hmm, it doesn't seem to work the way I expected

EDIT2: I made it work with just extend-exclude. "src/migrations/[0-9][0-9][0-9][0-9]_[!{manual}]*" does exactly what I want!

@charliermarsh
Copy link
Member

Nice! I think it probably didn't work as you'd expected because we do exclusions first, then inclusions, similar to Black (#3914 (comment)).

@tylerlaprade
Copy link
Contributor

tylerlaprade commented Aug 23, 2023

Update: It... sometimes works? I can't actually figure out what the logic is. 0118_contract_budget... is correctly excluded, but a bunch of random ####_alter_... are included, and I'm not sure why.

EDIT: Okay, I figured it out. contract starts with c, which is not in "manual". alter starts with a, which is. Based on the globset docs, I assumed I could use curlies to define a full string as a pattern instead of just a single character, but apparently it's still matching individual characters from inside the curlies.

image

@tylerlaprade
Copy link
Contributor

exclusions first, then inclusions

Shouldn't that actually make it behave the way I want? I want to first exclude the entire directory, then selectively include a few files from that directory.

@charliermarsh
Copy link
Member

@tylerlaprade - The way to think of it is: first, we iterate over all directories and files, removing any that match exclude; then, we pass the remaining files through include, and lint anything that matches. So if something is removed by an exclude, it can't be "re-added" by an include.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants