📎 New --changed argument

[ematipico]

Description

Biome now has support for VCS, which means that we can unlock new features that make the life of developers easier!

One of these features is the addition of a new argument called --changed to the CLI. When we pass this argument, Biome will execute check/format/etc. only to those files that were changed.

Technical details

I am still determining the implementation of this argument. There are two alternatives:

• execute the command via Command, we pass the proper arguments to the git program, and we read the output that was generated. Although I don't know yet what's the correct command/arguments to get an accurate output
• Use the crate git2. While this crate is phenomenal, its size is not small (207kb)

What do you guys think?

[simonxabris]

I was a lerna user for long and I really like its --since implementation. Basically you can provide any valid git ref to this command and it will roughly run the following git command: git diff --name-only <ref provided by user>...HEAD. This gives you the name of the files changed since that ref.

My proposal is this:

• Add a vcs.defaultBranch config option, where users specify the name for the default branch in their project.
• --changed will get the changes between defaultBranch and HEAD and only execute on those.

If there's no default branch configured, users will have to specify what they want to compare against with the CLI, for this we either:

• Add an optional argument to --changed, where they can specify a git ref to compare against, like --changed main
• Add a new argument called --since that does the same as above. Reason for this is imo --since main reads much better than --changed main.

Let me know what you think about this. If we can come to a conclusion on what to implement, I'm happy to contribute it!

[ematipico]

I love your proposal, I don't have anything to add! Feel free to take it and work on it. If you have any questions, just add a comment there and we'll try to answer

[simonxabris]

So as I'm implementing this, a couple of questions came up. My approach is based on how the gitignore files are handled. Basically I'm running the git diff command and extending (or creating) the files.include configuration field.

• First question, is this going to work or am I missing something?
• I've noticed the check command does not respect files.include field. If I have two files, src/test.js and src/test2.js and in the include field I specify src/test.js, when I run the CLI with the src/*.js pattern, the check command processes both files. The lint and format commands only process src/test.js. Is this a bug or is this intentional for some reason? If it's a bug I can file a new issue on this or I can just fix this as a part of this PR.

[ematipico]

So as I'm implementing this, a couple of questions came up. My approach is based on how the gitignore files are handled. Basically I'm running the git diff command and extending (or creating) the files.include configuration field.

* First question, is this going to work or am I missing something?

If with files.include you mean the workspace settings, then yes, it is going to work.

* I've noticed the `check` command does not respect `files.include` field. If I have two files, `src/test.js` and `src/test2.js` and in the include field I specify `src/test.js`, when I run the CLI with the `src/*.js` pattern, the `check` command processes both files. The `lint` and `format` commands only process `src/test.js`. Is this a bug or is this intentional for some reason? If it's a bug I can file a new issue on this or I can just fix this as a part of this PR.

Our CLI doesn't support glob patterns as input. Doing biome check ./src/*.js will result in expected errors.

[ematipico]

On the second, I'm not so sure about that, obviously I may be misunderstanding something

Internally, we don't try to be smart; we take the paths passed by the CLI and convert them into PathBuf. We don't throw an error:

biome/crates/biome_cli/src/execute/traverse.rs

Line 248 in 73da2fd

scope.spawn(ctx, PathBuf::from(input));

Then, we rely on the information that PathBuf APIs give us.

[simonxabris]

I think I cracked what's going on. So based on my testing, glob patterns are actually accepted and the bug I mentioned about the check command handling more files what's in the files.include property only happen when a glob pattern is used as an input to the CLI.

Output of `check` and `lint` with a supported pattern and with a glob pattern

So i guess the real bug is that the CLI does not error if a glob pattern is provided. I'll file this as a separate issue.

[simonxabris]

Internally, we don't try to be smart; we take the paths passed by the CLI and convert them into PathBuf. We don't throw an error

Oh I see, I just thought because you said Doing biome check ./src/*.js will result in expected errors. that I should've see an error using the CLI that way, but I didn't