Repository: terraform-docs/terraform-docs (go)

Issue:
output-template from CLI considers \n as literal and not line break
<!--
Thank you for helping to improve terraform-docs!

Please be sure to search for open issues before raising a new one. We use issues
for bug reports and feature requests. Please note, this template is for bugs
report, not feature requests. For more information, see the Contributing Guidelines
at https://git.io/JtEzg. Please find us at https://slack.terraform-docs.io/ for
questions, support, and discussion.
-->

### Describe the bug

<!--
Please let us know what behaviour you expected and how terraform-docs diverged
from that behaviour.
-->

When passing `--output-template` as CLI flag, `\n` is not processed as line break rather than literal strings. As such there's the following, misleading, error: `value of '--output-template' should contain at least 3 lines (begin comment, {{ .Content }}, and end comment)`

### How can we reproduce it?

<!--
Help us to reproduce your bug as succinctly and precisely as possible. Any and
all steps or script that triggers the issue are highly appreciated!

Do you have long logs to share? Please go to https://ghostbin.com and insert the
link here.
-->

```bash
$ terraform-docs markdown table --output-file README.md --output-template '<!-- BEGIN -->\n{{ .Content }}\n<!-- END -->'
Error: value of '--output-template' should contain at least 3 lines (begin comment, {{ .Content }}, and end comment)
exit status 1
```

Pull request:
Process \n as line break in --output-template flag
<!--
Thank you for helping to improve terraform-docs!

Please read through https://git.io/JtEzg if this is your first time opening a
terraform-docs pull request. Find us in https://slack.terraform-docs.io/messages/dev if
you need any help contributing.
-->

### Description of your changes

<!--
Briefly describe what this pull request does. Be sure to direct your reviewers'
attention to anything that needs special consideration.

We love pull requests that resolve an open terraform-docs issue. If yours does, you
can uncomment the below line to indicate which issue your PR fixes, for example
"Fixes #500":
-->

When passing `--output-template` as CLI flag, `\n` is not processed as
line break rather than literal strings, which caused a misleading error
of "value of '--output-template' should contain at least 3 lines".

Fixes #536

I have:

- [x] Read and followed terraform-docs' [contribution process].
- [x] All tests pass when I run `make test`.

### How has this code been tested

<!--
Before reviewers can be confident in the correctness of this pull request, it
needs to tested and shown to be correct. Briefly describe the testing that has
already been done or which is planned for this change.
-->

Added all the new test cases in `config_test.go`.

[contribution process]: https://git.io/JtEzg


Patch:
diff --git a/internal/cli/config.go b/internal/cli/config.go
index 31f9c52..ae78c35 100644
--- a/internal/cli/config.go
+++ b/internal/cli/config.go
@@ -249,6 +249,7 @@ func (o *output) validate() error {
 		return nil
 	}
 
+	o.Template = strings.ReplaceAll(o.Template, "\\n", "\n")
 	lines := strings.Split(o.Template, "\n")
 	tests := []struct {
 		condition  func() bool
@@ -291,24 +292,27 @@ func (o *output) validate() error {
 // ref: https://www.jamestharpe.com/markdown-comments/
 func isInlineComment(line string) bool {
 	switch {
-	// Markdown specific
-	case strings.HasPrefix(line, "<!--") && strings.HasSuffix(line, "-->"):
-		return true
-	case strings.HasPrefix(line, "[]: # ("):
-		return true
-	case strings.HasPrefix(line, "[]: # \""):
-		return true
-	case strings.HasPrefix(line, "[]: # '"):
-		return true
-	case strings.HasPrefix(line, "[//]: # ("):
-		return true
-	case strings.HasPrefix(line, "[comment]: # ("):
-		return true
-
 	// AsciiDoc specific
 	case strings.HasPrefix(line, "//"):
 		return true
+
+	// Markdown specific
+	default:
+		cases := [][]string{
+			{"<!--", "-->"},
+			{"[]: # (", ")"},
+			{"[]: # \"", "\""},
+			{"[]: # '", "'"},
+			{"[//]: # (", ")"},
+			{"[comment]: # (", ")"},
+		}
+		for _, c := range cases {
+			if strings.HasPrefix(line, c[0]) && strings.HasSuffix(line, c[1]) {
+				return true
+			}
+		}
 	}
+
 	return false
 }
 


Test patch:
diff --git a/internal/cli/config_test.go b/internal/cli/config_test.go
new file mode 100644
index 0000000..8cd7e3b
--- /dev/null
+++ b/internal/cli/config_test.go
@@ -0,0 +1,607 @@
+/*
+Copyright 2021 The terraform-docs Authors.
+
+Licensed under the MIT license (the "License"); you may not
+use this file except in compliance with the License.
+
+You may obtain a copy of the License at the LICENSE file in
+the root directory of this source tree.
+*/
+package cli
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConfigSections(t *testing.T) {
+	tests := map[string]struct {
+		sections sections
+		wantErr  bool
+		errMsg   string
+	}{
+		"OnlyShows": {
+			sections: sections{
+				Show: []string{sectionHeader, sectionInputs},
+				Hide: []string{},
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"OnlyHide": {
+			sections: sections{
+				Show: []string{},
+				Hide: []string{sectionHeader, sectionInputs},
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"BothShowAndHide": {
+			sections: sections{
+				Show: []string{sectionHeader},
+				Hide: []string{sectionInputs},
+			},
+			wantErr: true,
+			errMsg:  "'--show' and '--hide' can't be used together",
+		},
+		"UnknownShow": {
+			sections: sections{
+				Show: []string{"foo"},
+				Hide: []string{},
+			},
+			wantErr: true,
+			errMsg:  "'foo' is not a valid section",
+		},
+		"UnknownHide": {
+			sections: sections{
+				Show: []string{},
+				Hide: []string{"foo"},
+			},
+			wantErr: true,
+			errMsg:  "'foo' is not a valid section",
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			err := tt.sections.validate()
+
+			if tt.wantErr {
+				assert.NotNil(err)
+				assert.Equal(tt.errMsg, err.Error())
+			} else {
+				assert.Nil(err)
+			}
+		})
+	}
+}
+
+func TestConfigVisibility(t *testing.T) {
+	tests := []struct {
+		sections sections
+		name     string
+		expected bool
+	}{
+		{
+			sections: sections{},
+			name:     "header",
+			expected: true,
+		},
+		{
+			sections: sections{
+				Show: []string{"header"},
+				Hide: []string{},
+			},
+			name:     "header",
+			expected: true,
+		},
+		{
+			sections: sections{
+				Show: []string{"all"},
+				Hide: []string{},
+			},
+			name:     "header",
+			expected: true,
+		},
+		{
+			sections: sections{
+				Show: []string{},
+				Hide: []string{"inputs"},
+			},
+			name:     "header",
+			expected: true,
+		},
+
+		{
+			sections: sections{
+				Show: []string{},
+				Hide: []string{"header"},
+			},
+			name:     "header",
+			expected: false,
+		},
+		{
+			sections: sections{
+				Show: []string{},
+				Hide: []string{"all"},
+			},
+			name:     "header",
+			expected: false,
+		},
+		{
+			sections: sections{
+				Show: []string{"inputs"},
+				Hide: []string{},
+			},
+			name:     "header",
+			expected: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run("section visibility", func(t *testing.T) {
+			assert := assert.New(t)
+
+			visible := tt.sections.visibility(tt.name)
+			assert.Equal(tt.expected, visible)
+		})
+	}
+}
+
+func TestConfigOutput(t *testing.T) {
+	tests := map[string]struct {
+		output  output
+		wantErr bool
+		errMsg  string
+	}{
+		"FileEmpty": {
+			output: output{
+				File:     "",
+				Mode:     "",
+				Template: "",
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"TemplateEmptyModeReplace": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeReplace,
+				Template: "",
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"TemplateLiteralLineBreak": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeInject,
+				Template: fmt.Sprintf("%s\\n%s\\n%s", outputBeginComment, outputContent, outputEndComment),
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"NoExtraValidationModeReplace": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeReplace,
+				Template: fmt.Sprintf("%s\\n%s\\n%s", outputBeginComment, outputContent, outputEndComment),
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+
+		"ModeEmpty": {
+			output: output{
+				File:     "README.md",
+				Mode:     "",
+				Template: "",
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-mode' can't be empty",
+		},
+		"TemplateEmptyModeInject": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeInject,
+				Template: "",
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-template' can't be empty",
+		},
+		"TemplateNotContent": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeInject,
+				Template: fmt.Sprintf("%s\n%s", outputBeginComment, outputEndComment),
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-template' doesn't have '{{ .Content }}' (note that spaces inside '{{ }}' are mandatory)",
+		},
+		"TemplateNotThreeLines": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeInject,
+				Template: fmt.Sprintf("%s%s%s", outputBeginComment, outputContent, outputEndComment),
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-template' should contain at least 3 lines (begin comment, {{ .Content }}, and end comment)",
+		},
+		"TemplateBeginCommentMissing": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeInject,
+				Template: fmt.Sprintf("no-begin-comment\n%s\n%s", outputContent, outputEndComment),
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-template' is missing begin comment",
+		},
+		"TemplateEndCommentMissing": {
+			output: output{
+				File:     "README.md",
+				Mode:     outputModeInject,
+				Template: fmt.Sprintf("%s\n%s\nno-end-comment", outputBeginComment, outputContent),
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-template' is missing end comment",
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			err := tt.output.validate()
+
+			if tt.wantErr {
+				assert.NotNil(err)
+				assert.Equal(tt.errMsg, err.Error())
+			} else {
+				assert.Nil(err)
+			}
+		})
+	}
+}
+
+func TestIsInlineComment(t *testing.T) {
+	tests := []struct {
+		name     string
+		line     string
+		expected bool
+	}{
+		{
+			name:     "markdown comment variant",
+			line:     "<!-- this is a comment -->",
+			expected: true,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "<!-- this is not a comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "<!-- this is not a --> comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "this <!-- is not a comment -->",
+			expected: false,
+		},
+
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # (this is a comment)",
+			expected: true,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # (this is not a comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # (this is not a) comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "this []: # (is not a comment)",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]:#(this is not a comment)",
+			expected: false,
+		},
+
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # \"this is a comment\"",
+			expected: true,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # \"this is not a comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # \"this is not a\" comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "this []: # \"is not a comment\"",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]:#\"this is not a comment\"",
+			expected: false,
+		},
+
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # 'this is a comment'",
+			expected: true,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # 'this is not a comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]: # 'this is not a' comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "this []: # 'is not a comment'",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[]:#'this is not a comment'",
+			expected: false,
+		},
+
+		{
+			name:     "markdown comment variant",
+			line:     "[//]: # (this is a comment)",
+			expected: true,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[//]: # (this is not a comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[//]: # (this is not a) comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "this [//]: # (is not a comment)",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[//]:#(this is not a comment)",
+			expected: false,
+		},
+
+		{
+			name:     "markdown comment variant",
+			line:     "[comment]: # (this is a comment)",
+			expected: true,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[comment]: # (this is not a comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[comment]: # (this is not a) comment",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "this [comment]: # (is not a comment)",
+			expected: false,
+		},
+		{
+			name:     "markdown comment variant",
+			line:     "[comment]:#(this is not a comment)",
+			expected: false,
+		},
+
+		{
+			name:     "asciidoc comment variant",
+			line:     "// this is a comment",
+			expected: true,
+		},
+		{
+			name:     "asciidoc comment variant",
+			line:     "this // is not a comment",
+			expected: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			actual := isInlineComment(tt.line)
+			assert.Equal(tt.expected, actual)
+		})
+	}
+}
+
+func TestConfigSort(t *testing.T) {
+	tests := map[string]struct {
+		sort    sort
+		wantErr bool
+		errMsg  string
+	}{
+		"name": {
+			sort: sort{
+				By: sortName,
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"required": {
+			sort: sort{
+				By: sortRequired,
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"type": {
+			sort: sort{
+				By: sortType,
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+
+		"foo": {
+			sort: sort{
+				By: "foo",
+			},
+			wantErr: true,
+			errMsg:  "'foo' is not a valid sort type",
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			err := tt.sort.validate()
+
+			if tt.wantErr {
+				assert.NotNil(err)
+				assert.Equal(tt.errMsg, err.Error())
+			} else {
+				assert.Nil(err)
+			}
+		})
+	}
+}
+
+func TestConfigOutputvalues(t *testing.T) {
+	tests := map[string]struct {
+		outputvalues outputvalues
+		wantErr      bool
+		errMsg       string
+	}{
+		"OK": {
+			outputvalues: outputvalues{
+				Enabled: true,
+				From:    "file.json",
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"Disabled": {
+			outputvalues: outputvalues{
+				Enabled: false,
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"FromEmpty": {
+			outputvalues: outputvalues{
+				Enabled: true,
+				From:    "",
+			},
+			wantErr: true,
+			errMsg:  "value of '--output-values-from' is missing",
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			err := tt.outputvalues.validate()
+
+			if tt.wantErr {
+				assert.NotNil(err)
+				assert.Equal(tt.errMsg, err.Error())
+			} else {
+				assert.Nil(err)
+			}
+		})
+	}
+}
+
+func TestConfigProcess(t *testing.T) {
+	tests := map[string]struct {
+		config  func(c *Config)
+		wantErr bool
+		errMsg  string
+	}{
+		"OK": {
+			config: func(c *Config) {
+				c.Formatter = "foo"
+			},
+			wantErr: false,
+			errMsg:  "",
+		},
+		"FormatterEmpty": {
+			config: func(c *Config) {
+				c.Formatter = ""
+			},
+			wantErr: true,
+			errMsg:  "value of 'formatter' can't be empty",
+		},
+		"HeaderFromEmpty": {
+			config: func(c *Config) {
+				c.Formatter = "foo"
+				c.HeaderFrom = ""
+			},
+			wantErr: true,
+			errMsg:  "value of '--header-from' can't be empty",
+		},
+		"FooterFrom": {
+			config: func(c *Config) {
+				c.Formatter = "foo"
+				c.FooterFrom = ""
+				c.Sections.footer = true
+				c.isFlagChanged = func(s string) bool { return true }
+			},
+			wantErr: true,
+			errMsg:  "value of '--footer-from' can't be empty",
+		},
+		"SameHeaderFooterFrom": {
+			config: func(c *Config) {
+				c.Formatter = "foo"
+				c.HeaderFrom = "README.md"
+				c.FooterFrom = "README.md"
+			},
+			wantErr: true,
+			errMsg:  "value of '--footer-from' can't equal value of '--header-from",
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			config := DefaultConfig()
+			tt.config(config)
+			err := config.process()
+
+			if tt.wantErr {
+				assert.NotNil(err)
+				assert.Equal(tt.errMsg, err.Error())
+			} else {
+				assert.Nil(err)
+			}
+		})
+	}
+}