-
Notifications
You must be signed in to change notification settings - Fork 653
Conversation
✅ Deploy Preview for rometools canceled.
|
52efef4
to
ae1c8bf
Compare
ae1c8bf
to
472efd7
Compare
472efd7
to
3e195e1
Compare
3e195e1
to
2e77ea3
Compare
2e77ea3
to
c224f7e
Compare
c224f7e
to
ddd3b90
Compare
ddd3b90
to
235f844
Compare
235f844
to
0ba8b31
Compare
0ba8b31
to
ef73059
Compare
ef73059
to
566a4fc
Compare
566a4fc
to
b321ddf
Compare
b321ddf
to
2800be7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the traversal of a directory work. Does the implementation use the glob
patterns to filter out directories/files before calling into format / pull diagnostics of a single file ?
Our traversal works via a trait - among all our traits - called
Yes, this is done because we also want support cases where we directly interrogate a |
31c4e10
to
38d2c0d
Compare
2ed887e
to
79d5e69
Compare
I'll leave it to @leops to review this PR as he has more context. Would you mind doing some profiling. E.g. compare wall time of formatting/linting rome classic before/after your change? |
Using this PR
|
79d5e69
to
ee1c872
Compare
Summary
Closes #2827
Configuration
This PR adds support for ignoring files via configuration. The new field has been added to the
formatter
and thelinter
sections.deserialize_globals
andserialize_globals
have been renamed todeserialize_set_of_strings
andserialize_set_of_strings
, so I could apply the same functions for the fieldsglobals
andignore
;Matcher
I forked the create glob locally. I just copied the engine (parser) of the unix shell style patterns, as we are not interested in actual
glob
function, which does a traversal of the file system. The only changes that I applied to the crate are merely around clippy - it seems there's low activity on the code - and removed some tests. I won't be able to answer technical questions around this crate.I added license files and credits to the website.
I created a small wrapper called
Matcher
to add patterns and run the matcher against paths and strings. It also holds aHashMap
to track the already checked paths/strings.There's a catch with this crate: the crate doesn't support patterns like
"test.ts"
. I tested it myself and I think it's intentional. I added some basic check in theMatcher
struct to cover this cases, I figured that users might want to ignore single files.Workspace
The check is done in two places inside the workspace:
support_features
is_file_ignored
support_features
now returns a new struct which olds an optionalreason
that explains why the workspace doesn't support a certain file. Before, we were not distinguish the reason why a file couldn't be supported, and this caused some issue that are not fixed. If you check the snapshots, you might notice some deletions of some code that was out of place.We now use an enum called
UnsupportedReason
which tells the traversal/LSP why a file is not supported, and based on that they can react accordingly.is_file_ignored
is in the workspaceWorkspace
. This new function is now used in most of all functions that act on some files, I am not sure if I should extend the functionality also onget_syntax_tree
andget_control_flow_graph
.The usage of
is_file_ignored
has been placed before the parsing starts. There's no need to actually parse a file if it's ignored.When a file is ignored, we intentionally return an error. It's up to the clients to decide to ignore the error or deal with it. This is a pattern we already apply in case a file is not supported.
Test Plan
I added two new tests for the CLI.
I manually built the VSCode extension and made sure that I don't see diagnostics if a file is ignored. And also if the file doesn't get formatted.