alirezanet / Husky.Net

Git hooks made easy with Husky.Net internal task runner! 🐶 It brings the dev-dependency concept to the .NET world!
https://alirezanet.github.io/Husky.Net/
MIT License
632 stars 29 forks source link

Change `${matched}` to match over results of `git ls-files` #17

Closed atifaziz closed 2 years ago

atifaziz commented 2 years ago

${matched} casts too wide a net and scans/matches too many files by default. It's not usually what one wants because it can potentially end up returning hidden files and scanning the object database under .git/objects that could be very large. I would propose the default behaviour to return matches over files under version control, i.e. those returned by git ls-files. This way, one shouldn't have to be (too) careful with the variable.

alirezanet commented 2 years ago

Hi, yes I agree git ls-files makes more sense in 90 percent of scenarios but what if people want to do some cleanup using task runner (that requires ignored files), I couldn't find a solution for that yet. maybe I should use git ls-files by default when include and exclude are empty and use Directory.GetFiles when the task had some globs... In any case, I will exclude .git directory for sure

what do you think?

alirezanet commented 2 years ago

Or another solution could be adding a separate variable like ${git-matched}

atifaziz commented 2 years ago

but what if people want to do some cleanup using task runner (that requires ignored files), I couldn't find a solution for that yet.

It feels like Husky.Net is trying to be two things, Git hooks made easy and a general-purpose task runner? If the task runner is purely for facilitating the hooks then I don't expect people to be dealing with ignored files or running clean-up tasks.

maybe I should use git ls-files by default when include and exclude are empty and use Directory.GetFiles when the task had some globs...

That could be even more confusing. The behviour should could surprise people to see different results with and without the globs.

alirezanet commented 2 years ago

It feels like Husky.Net is trying to be two things,

Yes, I'm planning to support more

I don't expect people to be dealing with ignored files or running clean-up tasks.

I personally, use something like this in one of the projects, e.g. using the pre-push hook as an event to remove the obj/bin folder that is ignored.

That could be even more confusing

Except ignoring the .git folder that is necessary what is your suggestion?

alirezanet commented 2 years ago

I've added a ${committed} variable, probably this is what you looking for. it is using git ls-files output.

e.g

{
    "name": "test",
    "command": "bash",
    "args": [ "-c", "echo", "${committed}" ],
}
atifaziz commented 2 years ago

${committed} could be misleading since git ls-files will include staged files. Perhaps something as simple as ${git-ls-files} so that there is not guess work needed? In fact, I would say that ${matched} should be renamed to ${all}, ${files} or ${*} if you wish to keep it bound to the file system rather than Git.

Ultimately, if you really want to be open-ended then another idea would be to let the user define the source of the file list as another command. You could introduce the notion of hidden commands in task-runner.json. That is, any command whose name starts with . won't be run by default but could be used as a variable and its output becomes the file list. Here is how one could image a task runner JSON with two hidden commands (.git-ls-files and .files) and two test commands (test-git-ls-files and test-files) could look like:

{
   "tasks": [
      {
         "name": ".git-ls-files",
         "command": "git",
         "args": [ "ls-files" ]
      },
      {
         "name": ".files",
         "command": "find",
         "args": [ ".", "-type", "f", "-name", "*" ],
         "windows": {
            "command": "cmd",
            "args": [ "/c", "dir", "/s", "/b" ]
         }
      },
      {
         "name": "test-ls-files",
         "command": "printf",
         "args": [ "%s\\n", "${.git-ls-files}" ],
         "include": [ "**/*.cs", "**/*.vb" ],
         "windows": {
            "command": "cmd",
            "args": [ "/c", "echo", "${.git-ls-files}" ]
         }
      },
      {
         "name": "test-files",
         "command": "printf",
         "args": [ "%s\\n", "${.git-ls-files}" ],
         "include": [ "**/*.cs", "**/*.vb" ],
         "windows": {
            "command": "cmd",
            "args": [ "/c", "echo", "${.files}" ]
         }
      }
   ]
}

So you could provide some built-in ones (like ${staged} and ${lastCommit} that are already there) and then leave the stage open for other, more custom and advanced scenarios.

alirezanet commented 2 years ago

Considering your suggestions I changed the variable section in general. Check out Readme - Change logs

This issue is fixed via #18 PR. in v0.2.0.

thanks for the feedback.