optionally process comments from tf files as description #565

pbikki · 2021-09-19T07:05:48Z

Description of your changes

Fixes #552

This PR includes changes to optionally process comments from .tf files as description (when description is not provided in terraform inputs, outputs definition)

Adds flag read-comments

I have:

Read and followed terraform-docs' contribution process.
All tests pass when I run make test.

How has this code been tested

Added test TestReadComments()
Tested with make test
Tested manually

Manual Test

Create sample tf folder and file
mkdir /tmp/test-tf
cat <<EOF >> /tmp/test-tf/sample.tf
# some comment
variable "var1" {
}

# some comment
output "output-var1" {
  value = null
}
EOF
Tested with read-comments as false. Expected - description should be n/a (should not use comment (#some comment) as description)
▶ go run main.go --read-comments=false --show=inputs,outputs markdown /tmp/test-tf
Result:

Inputs

Name Description Type Default Required

var1 n/a any n/a yes

Outputs

Name Description

output-var1 n/a

Tested with read-comments as true. Expected - 'some comment' is used as description
▶ go run main.go --read-comments=true --show=inputs,outputs markdown /tmp/test-tf  
Result:

Inputs

Name Description Type Default Required

var1 some comment any n/a yes

Outputs

Name Description

output-var1 some comment

Cleanup
$ rm -r /tmp/test-tf

TBD

~~Config setting that maps to the new option added~~ Added new config setting read-comments
~~Tests when option is set to false. Currently few tests fail when option is set to false~~ set default to true (for backwards compatibility). Added new test cases to test when set to false and true

pbikki · 2021-09-20T17:44:44Z

@khos2ow based on our discussion, can you please review.

pbikki · 2021-09-20T17:54:08Z

I run make docs and it resulted in these changes in commit 26b860b3

When default is option is set to true here technically there shouldn't be any changes to the docs right ?

khos2ow · 2021-09-22T20:45:57Z

When default is option is set to true here technically there shouldn't be any changes to the docs right ?

You are absolutely correct on this, there shouldn't be any changes in docs/reference/ folder. This clearly indicates there's some flaw in the changes in this PR.

internal/terraform/options.go

khos2ow · 2021-09-22T21:28:15Z

OK, found the issue. It actually doesn't have anything to do with your implementation, the docs generator is faulty. Can you please add the same property (i.e. VarDescriptionFromComments or whatever you renamed it to) to https://github.com/terraform-docs/terraform-docs/blob/master/scripts/docs/generate.go#L177 ?

pbikki · 2021-09-22T21:32:44Z

https://github.com/terraform-docs/terraform-docs/blob/master/scripts/docs/generate.go#L177

That's exactly where I was looking. Assumed that for options that are not explicitly set, it'll use default. Got it now. Will update. Thanks!

khos2ow · 2021-09-22T21:56:50Z

Assumed that for options that are not explicitly set, it'll use default.

I'm actually in the process of refactoring Options which will solve this. It will be as you mentioned, anything now explicitly set in generate.go will take the default value.

pbikki · 2021-09-23T00:30:14Z

@khos2ow, added config ReadComments (cli flag read-comments) for now. Please review. Let me know if the added config setting should be propagated into printSettings and printSDKsettings

We discussed there might be some overlap with the description setting for hcl. But appears description solves a different purpose, so my guess is new flag added shouldn't be an issue. Below is an example when both flags are used.

▶ go run main.go tfvars hcl --description --read-comments=true <sample-tf-module>

# some comment
var1 = ""

▶ go run main.go tfvars hcl --read-comments=true <sample-tf-module>
var1 = ""

khos2ow

Please look at my comments below. Also please update the new config in:

README.md
docs/user-guide/configuration.md
docs/user-guide/configuration/settings.md

cmd/root.go

internal/cli/config.go

khos2ow · 2021-09-23T17:24:53Z

Let me know if the added config setting should be propagated into printSettings and printSDKsettings

This should be good as is and it's not necessary as this is an entirely internal config to load the module.

khos2ow · 2021-09-23T19:50:40Z

I realized in no point there's a test with ReadComments: false. It might be a good idea to create a test case, TestReadComments() in module_test.go, and for simplicity you can create internal/terraform/testdata/read-comments/ folder with one variable and one output with comments and test-cases of ReadComments both true and false.

pbikki · 2021-09-24T07:43:22Z

Updated docs (added new config setting)
Added TestReadComments() with testcases and testdata to test new config read-comments

khos2ow

Thank you @poojitha-bikki for all the efforts. lgtm!
would you please now squash your commits into one?

internal/terraform/module_test.go

…description fixes issue terraform-docs#552 - process description from comments Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - fix module tests Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - optionally read comments for output vars description Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - set default to true Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - run make docs Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - change option name Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - add option in doc generator; make docs Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - add config 'ReadComments' Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - Fix alphabetic order for 'ReadComments' setting Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - add read-comments in docs Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - add test for readcomments option Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com> - update 'read-comments' flag description Co-authored-by: Khosrow Moossavi <khos2ow@gmail.com> Signed-off-by: Poojitha Bikki <50849668+poojitha-bikki@users.noreply.github.com>

pbikki · 2021-09-27T16:03:04Z

Thank you @khos2ow for your guidance.

khos2ow · 2021-09-27T17:14:04Z

Thank you @poojitha-bikki for the great work!

pull-request-size bot added the size/M label Sep 19, 2021

pbikki force-pushed the issue-552-dev branch from da2cd4b to 59beea6 Compare September 19, 2021 07:17

pbikki marked this pull request as draft September 20, 2021 17:41

pbikki marked this pull request as ready for review September 20, 2021 17:41

pbikki marked this pull request as draft September 20, 2021 17:41

khos2ow added the config-file label Sep 20, 2021

khos2ow changed the title ~~[Draft] Issue 552 - optionally process comments from tf files for var description~~ optionally process comments from tf files as description Sep 20, 2021

pull-request-size bot added size/L and removed size/M labels Sep 20, 2021

khos2ow reviewed Sep 22, 2021

View reviewed changes

internal/terraform/options.go Outdated Show resolved Hide resolved

pull-request-size bot added size/S and removed size/L labels Sep 22, 2021

pull-request-size bot added size/L and removed size/S labels Sep 23, 2021

pbikki marked this pull request as ready for review September 23, 2021 00:36

khos2ow reviewed Sep 23, 2021

View reviewed changes

cmd/root.go Outdated Show resolved Hide resolved

internal/cli/config.go Outdated Show resolved Hide resolved

internal/cli/config.go Outdated Show resolved Hide resolved

internal/cli/config.go Outdated Show resolved Hide resolved

pbikki force-pushed the issue-552-dev branch 2 times, most recently from 28d28e4 to 51f63d1 Compare September 24, 2021 18:40

khos2ow approved these changes Sep 24, 2021

View reviewed changes

pbikki commented Sep 24, 2021

View reviewed changes

internal/terraform/module_test.go Show resolved Hide resolved

pbikki force-pushed the issue-552-dev branch from 51f63d1 to 045707b Compare September 24, 2021 23:08

khos2ow merged commit 4f698fb into terraform-docs:master Sep 27, 2021

optionally process comments from tf files as description #565

optionally process comments from tf files as description #565

Uh oh!

Conversation

pbikki commented Sep 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

How has this code been tested

Inputs

Outputs

Inputs

Outputs

Uh oh!

pbikki commented Sep 20, 2021

Uh oh!

pbikki commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khos2ow commented Sep 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

khos2ow commented Sep 22, 2021

Uh oh!

pbikki commented Sep 22, 2021

Uh oh!

khos2ow commented Sep 22, 2021

Uh oh!

pbikki commented Sep 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khos2ow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

khos2ow commented Sep 23, 2021

Uh oh!

khos2ow commented Sep 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pbikki commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khos2ow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pbikki commented Sep 27, 2021

Uh oh!

khos2ow commented Sep 27, 2021

Uh oh!

Uh oh!

pbikki commented Sep 19, 2021 •

edited

Loading

pbikki commented Sep 20, 2021 •

edited

Loading

khos2ow commented Sep 22, 2021 •

edited

Loading

pbikki commented Sep 23, 2021 •

edited

Loading

khos2ow commented Sep 23, 2021 •

edited

Loading

pbikki commented Sep 24, 2021 •

edited

Loading