fix(terraform_docs): Fix non-GNU sed issues, introduced in v1.93.0, as previous fix doesn't work correctly

[MaxymVlasov]

Put an x into the box if that apply:

• This PR introduces breaking change.
• This PR fixes a bug.
• This PR adds new functionality.
• This PR enhances existing functionality.

Description of your changes

Fixes #707

How can we test changes

repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: 70670818f5d14ba13c268dd366ecfa8374e88f42
    hooks:
      - id: terraform_docs

[robinbowes]

Suggested change: "${SED_CMD[@]}" and don't disable shellcheck

[MaxymVlasov]

Yeah, they're both (current and suggested variants) expanded to

trace: replace_old_markers terraform_docs terraform_docs_ main main
       terraform_docs.sh:49: sed -i -e 's/^<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->$/<!-- BEGIN_TF_DOCS -->/' README.md

trace: replace_old_markers terraform_docs terraform_docs_ main main
       terraform_docs.sh:50: sed -i -e 's/^<!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->$/<!-- END_TF_DOCS -->/' README.md

on Ubuntu. Not to sed -i'' -e but sed -i -e 🤔

[MaxymVlasov]

So let's wait till anyone who face original issue confirm that it will work

[robinbowes]

I'm on as Mac.

I can confirm it works with the quotes, and not without.

Test script:

old_insertion_marker_begin="Some text foo"
insertion_marker_begin="New Text"

set -x

if sed --version > /dev/null 2>&1; then
  SED_CMD=(sed -i'')
else
  SED_CMD=(sed -i '')
fi

"${SED_CMD[@]}" -e "s/^${old_insertion_marker_begin}$/${insertion_marker_begin}/" tt.txt

With quotes:

+ sed --version
+ SED_CMD=(sed -i '')
+ sed -i '' -e 's/^Some text foo$/New Text/' tt.txt

Without quotes:

+ sed --version
+ SED_CMD=(sed -i '')
+ sed -i -e 's/^Some text foo$/New Text/' tt.txt

[robinbowes]

If I replace sed with gsed, I get this output:

+ gsed --version
+ SED_CMD=(gsed -i'')
+ gsed -i -e 's/^Some text foo$/New Text/' tt.txt

Which works fine with GNU sed.

[MaxymVlasov]

test this please

repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: a76f03bfc4978741f37f79a6b4a86dc72078a78d
    hooks:
      - id: terraform_docs

[yermulnik]

Please see my suggestion re wrapping array elements in double quotes.

[MaxymVlasov]

okay, final (I hope) solution

repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: 70670818f5d14ba13c268dd366ecfa8374e88f42
    hooks:
      - id: terraform_docs

[yermulnik]

Suggested change

	if sed --version > /dev/null 2>&1; then SED_CMD=(sed -i''); else SED_CMD=(sed -i ''); fi
	if sed --version &> /dev/null; then SED_CMD=("sed" "-i''"); else SED_CMD=("sed" "-i" "''"); fi

It may also make sense to prepend all sed invocations with either \ (\sed) or command (command sed) to force executing a binary from PATH rather than a function or an alias 🤔

Also maybe replace if/else with ternary (sed --version &> /dev/null && SED_CMD=("sed" "-i''") || SED_CMD=("sed" "-i" "''"))?

[yermulnik]

Also probably for the GNU sed we can drop the '' bit: SED_CMD=("sed" "-i")

[MaxymVlasov]

for Mac will be next

trace: replace_old_markers terraform_docs terraform_docs_ main main
       terraform_docs.sh:47: SED_CMD=("sed" "-i" "''")

trace: replace_old_markers terraform_docs terraform_docs_ main main
       terraform_docs.sh:49: sed -i ''\'''\''' -e 's/^<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->$/<!-- BEGIN_TF_DOCS -->/' README.md

because on Ubuntu it will create README.md'' file, I don't think that it works

sed -i''\'''\''' -e 's/^<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->$/<!-- BEGIN_TF_DOCS -->/' README.md 

[robinbowes]

There's no need to wrap the individual array elements in quotes.

Using SED_CMD=(sed -i) makes sense for GNU sed as the '' gets dropped anyway.

[MaxymVlasov]

When without quotes we get what we need

trace: replace_old_markers terraform_docs terraform_docs_ main main
       terraform_docs.sh:47: SED_CMD=(sed -i '')

trace: replace_old_markers terraform_docs terraform_docs_ main main
       terraform_docs.sh:49: sed -i '' -e 's/^<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->$/<!-- BEGIN_TF_DOCS -->/' README.md

[yermulnik]

There's no need to wrap the individual array elements in quotes.

It helps to avoid empty elements to be "dropped".
It is also kinda common to wrap strings in quotes I guess.

[yermulnik]

It helps to avoid empty elements to be "dropped".

While they are still part of an array though. So nevermind me (given Max's tests show everything works as expected w/o quotes) =)

[yermulnik]

Please see my suggestion re wrapping array elements in double quotes.

[antonbabenko]

This PR is included in version 1.94.1 🎉