fix: Cleanup internal argument handling

[robinbowes]

ARGS and FILES are global arrays - no need to pass them as function arguments

[robinbowes]

@sergei-ivanov Can you please take a look at this. I've cleaned up the argument handling in terraform_docs so we simply use the global arrays ARGS and FILES rather than trying to pass them around internally.

[sergei-ivanov]

There's not really much point in shift anymore, now that we are using the globals.

Suggested change

	local -r terraform_docs_awk_file="$1" ; shift
	local -r terraform_docs_awk_file="$1"

[robinbowes]

There's not really much point in shift anymore, now that we are using the globals.

This is more of a safety net - it will throw an error if there is no arg.

[sergei-ivanov]

Same here probably:

Suggested change

	tfsec "${ARGS[@]}"
	tfsec ${ARGS[*]}

[robinbowes]

OK

[robinbowes]

Actually, I think using the quoted @ form is the correct approach here, and in terraform_docs.

Can you re-check with 3a68667 and let me know what you think?

[robinbowes]

FYI, it seems work OK for me:

 $ pre-commit try-repo -a ~/code/pre-commit-terraform terraform_docs
===============================================================================
Using config:
===============================================================================
repos:
-   repo: ../../../../pre-commit-terraform
    rev: 3a68667efc0f95f9c01db4a79aff33075697fc2e
    hooks:
    -   id: terraform_docs
===============================================================================
[INFO] Initializing environment for ../../../../pre-commit-terraform.
Terraform docs...........................................................Failed
- hook id: terraform_docs
- files were modified by this hook

[sergei-ivanov]

It does not work with args specified like this:

    hooks:
      - id: terraform_docs
        args:
          - '--args=document --indent 3'

...which is expected. But it does work when args are split into individual tokens:

    hooks:
      - id: terraform_docs
        args:
          - '--args=document'
          - '--args=--indent'
          - '--args=3'

I am not sure whether we would want to enforce passing the args one by one, or be more lenient like in the 1st example.

[robinbowes]

I think that's a cleaner solution than trying to cope with eg. '--args=document --indent 3'

[sergei-ivanov]

Both options are fine by me. If we settle on option 2, I think we need a section in the documentation/readme with a usage example for additional arguments (feel free to use the example above). Simply because it is not obvious at all at the moment.

[robinbowes]

Agreed - the docs need updating.

Is your example correct, though? Wouldn't it be this:

hooks:
      - id: terraform_docs
        args:
          - 'document'
          - '--indent=3'

I think the other examples in the README may be wrong too.

[sergei-ivanov]

The one above is correct and tested. One needs to prepend each argument with --args= prefix. I guess that is because we want to distinguish the arguments to terraform-docs from the list of files passed in by pre-commit.

So the arguments should be passed like this:

      - id: terraform_docs
        args:
          - '--args=document'
          - '--args=--indent'
          - '--args=3'

[robinbowes]

OK, so I think we're saying we should use one arg per line ie. option 2 but that we should make sure we update the documentation to be clearer.

[jpatallah]

When is the slated to be merged in? Our tfsec exclusion parameter isn't being read properly and this fix is very important!

[github-actions]

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

[yermulnik]

Suggested change

	(( index+=1 ))
	(( index++ ))

[MaxymVlasov]

Nope, we should pass them to functions, because it explicitly mentions which vars go to function.