WIP: Add support for fetching the module header from any file

[mvisonneau]

No description provided.

[metmajer]

Hi @mvisonneau.

Thank you for your interest in this project.

What is the intention behind your contribution? According to your source code, the file which is enumerated last and for which len(comments) > 0 determines the comment of the resulting document. I am not sure if this is better than the existing solution.

Please note that with the release of v0.4.0, the project has undergone major refactoring to facilitate future community contributions like yours. Likewise, in the absence of tests, maintaining a high-quality codebase was close to impossible. Also, this has now changed. If you're interested in making a contribution, please consider our Contribution Guidelines.

[mvisonneau]

👋 hey @metmajer, thanks for reviewing my MR. The intent was to be able to set the headers from any files in the project. At the time of my change, only main.tf could be used as filename for holding those headers in order to get them rendered properly. I'll have a look if this is something that is now included in the latest release 👍

[metmajer]

Hey @mvisonneau. I've recently gone over the implementation and the change has not yet landed in the codebase. If I understand correctly, then you assume that only 1 .tf file contains the module's comment and this should be parsed into the resulting documentation.

I wonder if this assumption is always correct. I am working on a module where each of main.tf, variables.tf and outputs.tf contain a comment block in the beginning. With your contribution, the resulting documentation would include the header of the variables.tf (assuming that file enumeration happens alphabetically). Since that's clearly not the intended behavior, we would be better of following the convention where a module's comments are assumed to be present in main.tf.

Thoughts?

[mvisonneau]

👋 indeed, what do you think of getting a flag to configure which file to use that defaults to main.tf otherwise ? 🤔

[metmajer]

Hi there, @mvisonneau! This sounds like a good idea. Personally, I think we should take the module comment from a single file only and not try to have it collected from multiple files. What do you think about a flag --with-module-comments-from which defaults to main.tf?

Let's see if @jmcmaster05 has some thoughts to share?

[jmcmaster05]

I like the idea of a single block as multiple comment blocks could prove to be an interesting mash up.

main.tf seems the appropriate default but in practice I would probably make use of the --with-module-comments-from flag to point to a readme.tf to keep all the hard coded readme descriptions separate (temporarily remove the encapsulating '/' '/' to allow for markdown lint etc). Or in a simple project just putting the small comment block in the main.tf.

The more we make use of modules in a given project the more we put everything (vars.tf, provider.tf, etc) into a single main.tf as fewer lines of code make multiple files more annoying than helpful. But I see many of us through the growing/learning executing terraform-docs md *.tf on multiple terraform files so a little more control over were that comment block lives would be helpful.

[metmajer]

@jmcmaster05 thanks for sharing your thoughts!
@mvisonneau I'd love to have you support us with this. If you need help on the details, let me know!

[mvisonneau]

Refactored / rebased in order to get it working. Not a 100% pleased with the way I've done it, I'm open for suggestions! Got some additional tests as well 👍

[metmajer]

Thanks @mvisonneau for your contribution. I will look into this and provide feedback in the next days!

[metmajer]

Hi @mvisonneau!

I've noted from your contribution that we need a better means for passing settings from the command line into the application. Adding these settings into the signature of each function is hacky and I'd rather strive for a clean solution. I've already created issue #83 to support this case. This will take a little time though.

Additionally, we should revise your current testing strategy to avoid having to touch all existing tests. 👍

[mvisonneau]

yep definitely 👍 thanks!

[metmajer]

Hey @mvisonneau! Apologies for having you wait so long for an update. We are already working on integrating support for Terraform 0.12. As it turns out, support for reading out comments is not available in HCL 2.0 (see hashicorp/terraform#1960).

I'll defer this PR for now and see what we can do going forward. Thanks!

[morgante]

@metmajer How is 0.12 support coming along?

[bikochan]

I was thinking about adding that same support until I found this PR.
Is there any update on this?
I have lots of existing modules documentation I'd like to convert to terraform-docs w/o renaming files to main.tf if possible.

[metmajer]

@bikochan if you look at #62, you'll find that Terraform 0.12 doesn't easily support parsing of comments at all. Nevertheless, we're on it.

[bikochan]

I had checked (see hashicorp/terraform#1960) that you referenced earlier in this PR comments but it was about some other issue and had been merged; that's why I asked TBH.
So you're waiting for a way to update for tf 0.12 too. Fair enough. Thanks for the update.

[mwarkentin]

Sounds like support for comments has been removed as of support for 0.12, so this can probably be closed?

[khos2ow]

With the recent changes we've made to this repo, this proposal became valid once again and considering migration to cobra a while ago this is absolutely doable.
My only concern at the moment is that rebasing this PR will be very difficult and starting from the scratch might be easier. We can circle back to it in v0.9.0.

[khos2ow]

Closing this in favor of #217