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

exclude pattern with * in file name incorrectly matches entire file path #6262

Open
Tracked by #458
DetachHead opened this issue Aug 2, 2023 · 9 comments · May be fixed by #11139
Open
Tracked by #458

exclude pattern with * in file name incorrectly matches entire file path #6262

DetachHead opened this issue Aug 2, 2023 · 9 comments · May be fixed by #11139
Assignees
Labels
cli Related to the command-line interface needs-decision Awaiting a decision from a maintainer

Comments

@DetachHead
Copy link

DetachHead commented Aug 2, 2023

ruff --exclude foo*.py

this incorrectly excludes the file foo/bar/baz.py.

since excludes takes globs (as mentioned here), i would expect it to only match python files with names starting with foo.

i'm on windows (which uses \ as path separators instead of /), that may be related

@charliermarsh
Copy link
Member

We use globset which lets * match the path separator by default, but is configurable to disable that behavior (https://docs.rs/globset/latest/globset/#syntax).

@zanieb - could use a second opinion, any objection to changing that?

@charliermarsh charliermarsh self-assigned this Aug 2, 2023
@zanieb
Copy link
Member

zanieb commented Aug 2, 2023

What are you expected to do to get the desired behavior without literal_separator set? What's the point of ** then?

I'm a little confused that this is the default in globset @BurntSushi is probably best for a second opinion :D

It'd be a breaking change for us to change this, but I think it'd probably be best to match Unix shell behavior.

@BurntSushi
Copy link
Member

On mobile so apologies for short reply.

Globbing is pretty tricky because it depends on the context in which it is run. For example, at a shell, you might expect *.py to only match Python files in the current directory, but in a .gitignore file you would expect (and is what happens) *.py to exclude both foo.py and bar/foo.py. In the gitignore world, you would restrict your glob to the current directory (relative to the .gitignore file) by prefixing the glob with a /. So, /*.py would match foo.py but not bar/foo.py.

So whether *.py is supposed to match a path separator or not depends a lot on the context in which a glob is applied. But generally speaking, for exclude rules I would try to follow what gitignore does. (With that said, I haven't looked at in detail the context here, so take what I'm saying with a grain of salt.)

@charliermarsh
Copy link
Member

Thanks! I'm impressed that you can manage code formatting on mobile.

@charliermarsh
Copy link
Member

We have kind of a weird thing going on, which I think I followed from Flake8 or Black. When given a pattern like foo*.py, we match against both the basename and the absolute path (when testing to exclude a given file or directory).

So it may make sense to turn on literal_separator, since we'd still match *.py on the cases described above. Or we may want to reconsider our behavior here more holistically.

@BurntSushi
Copy link
Member

Or we may want to reconsider our behavior here more holistically.

👍

@charliermarsh charliermarsh added cli Related to the command-line interface needs-decision Awaiting a decision from a maintainer labels Aug 2, 2023
@tylerlaprade
Copy link
Contributor

Thanks! I'm impressed that you can manage code formatting on mobile.

I'm not sure about Android, but if you long-press the ' key on the standard iPhone keyboard it will give you a contextual menu of other apostrophe-like characters, including `.

@KotlinIsland
Copy link
Contributor

Thanks! I'm impressed that you can manage code formatting on mobile.

I'm not sure about Android, but if you long-press the ' key on the standard iPhone keyboard it will give you a contextual menu of other apostrophe-like characters, including `.

On Android, the long press variations for ' do not include `, but you can instead do:
?123 -> =\< -> `

@JP-Ellis
Copy link

JP-Ellis commented Nov 3, 2023

Came here because I was having an issue with the globs unexpectedly including subdirectories.

In my case, I am trying to exclude tests/*.py (which contains an old codebase which will be deprecated) while including tests/v3/*.py. Following the documentation:

  • Relative patterns, like directory/foo.py (to exclude that specific file) or directory/*.py (to exclude any Python files in directory). Note that these paths are relative to the project root (e.g., the directory containing your pyproject.toml).

I added:

[tool.ruff]
extend-exclude = ["tests/*.py"]

I was surprised to find out that this excluded all Python files in tests/ and subdirectories thereof. As pointed out above, it brings into question the purpose of having both * and **. Being a glob pattern, Ruff's behaviour also doesn't match a standard shell glob (e.g., ls tests/*.py does not list files in tests/v3/)

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 needs-decision Awaiting a decision from a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants