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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run files cmd with root parameter #292

Open
KubaJastrz opened this issue Jun 29, 2022 · 1 comment · May be fixed by #607
Open

Run files cmd with root parameter #292

KubaJastrz opened this issue Jun 29, 2022 · 1 comment · May be fixed by #607
Labels
feature request A new lefthook feature description

Comments

@KubaJastrz
Copy link

KubaJastrz commented Jun 29, 2022

Files array is computed by running a files command from working directory. But run is run from filepath.Join(r.repo.RootPath, command.Root). This makes a weird looking configuration in case of using custom scripts for both options:

# consider nested packages/test/lefthook.yml
pre-push:
  commands:
    eslint:
      root: 'packages/test/'
      files: scripts/get-files-push               # this is relative to working directory (repo root)
      run: ../../scripts/lint-fix eslint {files}  # this is relative to packages/test

In my opinion files should also be computed based on root path.

Source code:

if runtime.GOOS == "windows" {
commandArg := strings.Split(command, " ")
cmd = exec.Command(commandArg[0], commandArg[1:]...)
} else {
cmd = exec.Command("sh", "-c", command)
}

root := filepath.Join(r.repo.RootPath, command.Root)
r.run(name, root, args)

@mrexox mrexox added the feature request A new lefthook feature description label Nov 14, 2022
@jameslporter
Copy link

jameslporter commented Jan 10, 2024

Love lefthook!! I just dove in a day or so ago. This was the only real pain point for me. We have a CRA app in client directory and server node code in the root level folders. Below is the run command I came up with to fix the paths by removing the root level of the path. If someone needs this but deeper into the directory structure adjust the cut -d'/' -f2- part to make the number be +1 of how many directories to lop off.

run: fixedFiles=$(for staged_file in ${staged_files}; do echo $(echo $staged_file | cut -d'/' -f2-); done;); cd client; prettier --ignore-unknown --write $fixedFiles && eslint --max-warnings 0 $fixedFiles && git add $fixedFiles

Unrelated but thought I'd share this idea while I'm here. I made a post-merge action that globs on lefthook.yml to trigger lefthook install. This way new developers work out of the box after npm i from a fresh clone and existing developers will be kept up to date as the config evolves.

post-merge:
  commands: 
    lefthook:
      glob: "lefthook.yml"
      run: npm exec lefthook install

only other minor feedback is occasionally it spits characters out into my terminal after running npm exec lefthook run pre-commit. Not very consistent so repro is difficult of course. Anyways, thanks for this contribution and plus one to this feature request.

@mrexox mrexox linked a pull request Jan 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new lefthook feature description
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants