Skip to content

Conversation

TECHNOFAB11
Copy link
Contributor

@TECHNOFAB11 TECHNOFAB11 commented Jul 9, 2025

⚡ Summary

Adds an env var "LEFTHOOK_CONFIG" to specify the config file, can be relative or absolute.

☑️ Checklist

  • Check locally
  • Add tests
  • Add documentation (where to put that? the entry in --help might be enough?)

@TECHNOFAB11 TECHNOFAB11 requested a review from mrexox as a code owner July 9, 2025 13:27
@mrexox
Copy link
Member

mrexox commented Jul 9, 2025

Could you share the problem you’re trying to solve, please?

@TECHNOFAB11
Copy link
Contributor Author

Sure, integration with Nix. Generating the config file from Nix, then wrapping lefthook to use that (eg lefthook --config /nix/store/....yml $@). Otherwise I'd have to generate the config file from Nix and always have to keep it in sync and in the git repo, which isn't great

@mrexox
Copy link
Member

mrexox commented Jul 10, 2025

@TECHNOFAB11 I see, so maybe a ENV variable would be a better choice? Lefthook can be run from within a git hook and there you can’t pass a --config parameter. ENV variable would be easier to tweak. WDYT?

@TECHNOFAB11
Copy link
Contributor Author

@mrexox I generate the git hook files myself so both a cli arg or env var would be fine.
Would you rather just have the env var or have both, by defaulting the cli arg to the env var?

@mrexox
Copy link
Member

mrexox commented Jul 10, 2025

@TECHNOFAB11 I’d better use just the ENV var because it’s more flexible and requires less code to implement. Also lefthook is kind of ”hidden” tool which runs implicitly by Git hooks, so I assume that ENV var would have more users.

If there’s LEFTHOOK_CONFIG var set, let’s think that it overwrites any other configs (even lefthook-local.yml).

By the way, do you need help implementing this?

@TECHNOFAB11
Copy link
Contributor Author

@mrexox sounds good, will modify the PR later today to use an env var instead 👍

@TECHNOFAB11 TECHNOFAB11 changed the title feat: add cli flag to specify config path directly feat: add env var to specify config path directly Jul 10, 2025
@TECHNOFAB11
Copy link
Contributor Author

Conflicts resolved 👍

@TECHNOFAB11
Copy link
Contributor Author

Seems like a line got lost when fixing the conflicts, whoops, fixed :D

@mrexox mrexox merged commit 47ba9db into evilmartians:master Jul 11, 2025
17 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants