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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃搸 Support {a,b} glob pattern syntax for includes/excludes #2986

Open
dyc3 opened this issue May 25, 2024 · 6 comments 路 Fixed by #2989
Open

馃搸 Support {a,b} glob pattern syntax for includes/excludes #2986

dyc3 opened this issue May 25, 2024 · 6 comments 路 Fixed by #2989
Milestone

Comments

@dyc3
Copy link
Contributor

dyc3 commented May 25, 2024

Summary

We want to support {a,b} pattern matching syntax for includes and excludes. It will allow us to more comprehensively support all the patterns that the Editorconfig spec defines (although not completely).

Specifically, this syntax is to indicate that in the pattern {a,b}, there is a comma separated list of patterns that are applicable in that position.

@dyc3 dyc3 changed the title 馃搸 Support {a.b} glob pattern syntax for includes/excludes 馃搸 Support {a,b} glob pattern syntax for includes/excludes May 25, 2024
@ematipico
Copy link
Member

ematipico commented May 26, 2024

The pattern logic is an internal fork of glob: https://docs.rs/glob/latest/glob/struct.pattern.html

Other than that, unfortunately I can't give you more information, we will have to figure it out ourselves.

Regarding the recursion, I think it's safe to assume that we don't want to support nested patterns deeper than one level:

  • {*.ts, *.js} yes
  • {{*.ts, *.js}} no

Even other libraries that offer Unix glob parsing and matching don't support nested patterns.

If we set this as a goal, I think recursion isn't needed. Or we could use it, and it should not put too much memory pressure.

@Sec-ant
Copy link
Member

Sec-ant commented May 26, 2024

FYI, there're some pre-discussions here: #2620

@Conaclos Conaclos added this to the Biome 2.0 milestone May 26, 2024
@Conaclos
Copy link
Member

I think we can set this task for Biome 2.0 because this will require a breaking change.

@ematipico
Copy link
Member

@Conaclos why was this PR merged if the milestone is set for Biome 2.0?

@eMerzh
Copy link
Contributor

eMerzh commented Jul 4, 2024

does this still need to be open?

@dyc3
Copy link
Contributor Author

dyc3 commented Jul 4, 2024

Yes. The code for this is already implemented, but it's placed behind a flag. This issue should be closed when that flag is removed in biome 2.0.

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 a pull request may close this issue.

5 participants