Yep, we're open for contributions. We welcome and encourage them. Especially those of such a brave kind 👍🏻 You may take the https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md as a base point. While it is not quite Windows-aware I guess, it may probably help you with the direction.

I'd like to see those shell scripts converted to Python

Well, if you'll provide at least the same execution speed as in the shell, we will check your PR. If it will work slower - from README:

We highly recommend using WSL/WSL2 with Ubuntu and following the Ubuntu installation guide. Or use Docker.

I'll recommend building PoC on something simple, like hooks/terraform_fmt.sh, test it execution speed on big repo via pre-commit run -a and compare it with current realization, to understand is it worth it at all.

If you have no big repo - try to test on a tiny one but you'll need aware of fractions of a second and test it many times in a row. And when you're ready - send PoC PR - I have a huge repo where I'll test it :)

Sounds good. Just wanted to make sure you'd be okay with taking on Python as a dependency before attempting a prototype.

To me at least, Python makes more sense of a dependency than Bash does since any environment running pre-commit already has Python but not necessarily Bash.

I'll give it a try.

I'm noticing that the hooks_performance_test.sh seems biased towards hooks of language script.

These times include the overhead of pre-commit itself creating a virtual environment for the python based scripts. This is a one-time event and perhaps shouldn't be counted in the performance testing. It even says:

[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...

I would like to continue, but need to know that you would accept a solution that is performant if you concede the installation time. Unfortunately this is inherent to how pre-commit try-repo works, so maybe I need to do some manual installation and use a command other than try-repo for the hooks_performance_test.sh

no-op tests

I changed both terraform_fmt of language script as well as terraform_docs_replace of language python to immediately exit 0 / sys.exit(0). So they actually do nothing, but Python will be penalized like 1.5 seconds

5 runs 'dummy script'

5 runs 'dummy python'

Just wanted to make sure you'd be okay with taking on Python as a dependency

As you already mentioned, Python is a hard dependency for pre-commit framework that pre-commit-terraform is based on. So we actually can't decline Python =)

Python makes more sense of a dependency than Bash

If you feel you can replace all the shell related bits with Python, as @MaxymVlasov already mentioned above we'll be keen to see whether this can be viable and feasible solution.
It's not about just Bash as there are other CLI utilities in use also. Not talking about the "generic" CLI tools like terraform, infracost, checkov, terraform-docs, tflint, and others, which you'd probably not rewrite in Python, but which you would need to implement logic for if you're targeting to support not just Linux/macOS, but also Windows.

@yermulnik I'm happy to discuss this here as suggested.

I am confused by asking me to look at man 1 bash because while the hook is implemented in bash, these --env-vars are clearly not interpreted via Bash itself.

Use the following args...

  - id: terraform_fmt
    args:
      - --env-vars=DEBUG_START="start
      - --env-vars=DEBUG_END=end"
      - --env-vars=DEBUG_MID=mi"d
      - --env-vars=DEBUG_BOTH="both"
      - --env-vars=DEBUG_BARE=bare
      - --env-vars=DEBUG_MSG1="This is not how "bash" works"
      - --env-vars=DEBUG_MSG2="this is how \"bash\" works"

... and you'll see the behavior differences between what _common.sh supports and what Bash supports.

My point is, man 1 bash is not the place to look at what these hooks support.

For the purposes of this feature, I just need to know what to support. I can implement it to work exactly the same. I'd actually prefer if these hooks didn't fully support Bash interpretation because that would make this feature impossible. 2 lines of simple bash should be about 2 lines in any language.

So... finally, I'll just ask my question: should my Python implementation exactly mimic the behavior in the _common.sh column in the above table?

So... finally, I'll just ask my question: should my Python implementation exactly mimic the behavior in the _common.sh column in the above table?

For that case, better choose how Bash would handle it interpretation. how _common.sh exports it looks more like a bug than a feature.

I am confused by asking me to look at man 1 bash because while the hook is implemented in bash, these --env-vars are clearly not interpreted via Bash itself.

They are: https://github.com/antonbabenko/pre-commit-terraform/blob/master/hooks/_common.sh#L536 (not specifically interpreted, which makes that function do half of the task apparently, though still those vars are handled by Bash to get them exported into env).

My point is, man 1 bash is not the place to look at what these hooks support.

All of the existing hooks rely upon Bash specifically (apart from terraform_docs_replace.py). Bash is the interpreter of those shell scripts.

For the purposes of this feature, I just need to know what to support.

If I got it right, the gist of this feature is to re-implement hooks in Python. For this goal I'd say you wanted to support the result (altogether with existing user-facing options and features) — if you can replicate what benefits these hooks bring to our users, then the goal is accomplished IMHO.

I'd actually prefer if these hooks didn't fully support Bash interpretation because that would make this feature impossible.

I think the idea must be to re-implement rather than to replicate. Hence you're through the greenfield and have all the options to re-implement the way it is better for users.

So... finally, I'll just ask my question: should my Python implementation exactly mimic the behavior in the _common.sh column in the above table?

I'm more than certain — again — you should be targeting to implement the result, rather than behavior.

PS: Export vars the way they should be exported from within any language. As @MaxymVlasov already mentioned, the _common.sh way to do it is more of a hacky rather than a proper. We had to had a balance between full interpretation (which would imply eval which @MaxymVlasov is against of and this makes perfect sense) and the missing feature that our users asked for.