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

Avoid regex in fnmatch crate #191

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

Avoid regex in fnmatch crate #191

wants to merge 4 commits into from

Conversation

livingsilver94
Copy link
Contributor

@livingsilver94 livingsilver94 commented Mar 26, 2024

This PR replaces the regex backend with an in-house glob parser.

The parser is two-pass:

  1. Tokenize the pattern string in text, escape character and other control characters that are part of the glob syntax.
  2. Rework the raw tokens above as a list of text and glob tokens. These two kinds of tokens are the only ones useful to match a file path.

Pattern then walks through a given file path and returns the result.

By avoiding regex we mainly avoid to fall victim of unescaped characters that may compromise globbing. We probably also increase performance, but I haven't run any benchmark and it'd be likely a negligible advantage, since file paths are short strings.

Extensive testing is very welcome. Ideally, we should fuzzy testing this too.

Limitations

  • Contrary to the globbing of common knowledge, this implementation does not support character groups ([abcd]). It only supports * and ?.
  • It currently doesn't support consecutive globs. Text and glob tokens must always come one after the other. Now two * wouldn't be useful, but two consecutive ? may be. Should such need arise, I'll work on supporting it.
  • It has the concept of chars, not UTF-8 graphemes. Non-ASCII characters in paths are uncommon, but I could use a crate for walking graphemes if needed.

@livingsilver94
Copy link
Contributor Author

Clippy complains about code I did not touch ._. please ignore it.

@ermo ermo added the type: enhancement New feature or request label Apr 16, 2024
@livingsilver94 livingsilver94 force-pushed the fnmatch-simplify branch 2 times, most recently from 703e701 to c943499 Compare May 10, 2024 17:16
@livingsilver94
Copy link
Contributor Author

Rebased. spellcheck yells at me because I named a variable matc :|

@livingsilver94 livingsilver94 marked this pull request as ready for review July 26, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants