-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
📎 Use biome_glob
for the Biome config file
#4611
Comments
Good idea!
If we make this change with the introduction of Biome 2.0, do we need to keep the deprecated fields at all? As long as |
I wouldn't want to keep two fields, it brings too much maintenance burden for us. I would go for a clean cut. That's what breaking changes are for. What we can try to do as much as we can with the migration command, it's possible we won't be able to do that and it's fine. We will work with users and early adopters to fix possible bugs. We will explain the entity of the breaking change to users and provide a migration path . |
This is a good point.
I think it is possible to cover most of the cases and emit a diagnostic for cases we cannot convert. |
Best migration path ever :) |
One note of caution: Biome supports Because it's hard to change the pattern syntax afterwards (as evidenced by this issue), we should probably make sure to get it right "this time". Especially because there are some subtleties regarding user expectations as well as a lot of corner cases involved (which is the reason why this comment got as long as it is now...). Here's an overview of the syntax & behavior differences (and similarities) I could find: (Note: I didn't compare with
Based on all this research, I've come to the following conclusions:
In conclusion, I would recommend making the I would even go as far as recommending contributing a Rust version of the Even if it is decided to not go down that route, it would still be beneficial to:
Footnotes
|
I think we should use a different glob type for Also, I plan to add support of ASCII character classes and non-empty non-nested literal alternations
The difference is very small. I could personally use exactly the same syntax for consistency and simplicity. Some months ago I started a glob library to cover |
Fair point, given the potential for future changes, which I haven't taken into account.
Well, then I think it would be best to skip the implicit |
Description
Biome currently uses a fork of the glob crate. This crate has known shortcomings that can be considered as bugs by users. See #2421 and #3345.
For instance the globs
src/**
and*.txt
are currently interpreted as**/src/**
and**/*.txt
.Also, we would like in the future to support convenient syntaxes such as #2986.
In the past months, several linter or assist rules required globs. Instead of using our
glob
fork, I introduced biome_glob::Glob (Previously known asRestrictedGlob
). This currently encapsulates theglobset
crate and provides glob list with exceptions (negated globs).Our current Biome configuration file uses globs in
include
andignore
fields.Switching from our glob fork to
biome_glob
could cause breakage on users.This is why I propose to keep the
include
andignore
fields bound to our fork ofglob
, to deprecate them, and to introduce a newfield
includes
(include
with a trailing s).includes
will use a glob list with exceptions.Also, to make top-level
includes
more accessible, to propose to place it directly at the root of the configuration (i.e.includes
instead offiles.include
/files.ignore
). See the migration example.Tasks:
includes
include
/ignore
and provide an automated migration (usingbiome migrate
) to ease migration frominclude
/ignore
toincludes
.Implementation notes
Our file crawler currently doesn't traverse a directory if the directory is ignored in a
ignore
file.I introduced a method
biome_glob::CandidatePath::matches_directory_with_exceptions
to preserve this behavior.Migration example
The migration tool will have in charge of translating
include
/ignore
patterns intoincludes
.For instance the globs
src/**
and*.txt
have to be translated as**/src/**
and**/*.txt
.Discussion
includes
andinclude
are really close (just a trailing s creates the difference). This could cause some confusion.An alternative name could be
files
. Unfortunately this conflicts with the top-levelfiles
config.If we want to use
files
we will have to deprecate the top-levelfiles
object and to find an alternative way of configuring the files options.Maybe the confusion is ok because
include
will be deprecated anyway and eventually removed.The text was updated successfully, but these errors were encountered: