Update pre-commit-hook.rb so that we run msftidy_docs.rb on PR submissions that modify documentation

[gwillcox-r7]

This updates the pre-commit-hook.rb file that is already run in our lint GitHub Actions via https://github.com/rapid7/metasploit-framework/blob/master/.github/workflows/lint.yml#L59-L63

This will ensure that both msftidy.rb and msftidy_docs.rb are run on PR submission and should prevent a lot of issues that we currently have with people running rubocop on their PRs to fix the module but not running msftidy_docs.rb on their PR's documentation and then us getting PRs that have different documentation formats.

I also updated msftidy_docs.rb since a lot of errors are currently labeled as warnings instead of errors so this update will make it easier to properly block PRs that don't conform to the expected format whilst also allowing exceptions for long lines and modules that might not have an Options section.

Verification

List the steps needed to make sure this thing works

• [ ] Check out this PR and modify a random file in documentation/modules/.
• [ ] Run git add <file you modified>
• [ ] Run tools/dev/pre-commit-hook.rb
• [ ] Verify that this has msftidy_docs.rb run on the file you modified.
• [ ] Double check if we need to update our code in msftidy_docs.rb so this integration can work more appropriately.

[h00die]

this makes me real happy :)

[gwillcox-r7]

Woops requested review but need to fix a merge conflict first. Let me fix that and then we can have a discussion about any changes that may need to be done here.

[adfoster-r7]

Still pondering this PR 🤔

This might introduce a similar problem to https://github.com/rapid7/metasploit-framework/issues/17582 - where it might be annoying to contributors having to update legacy documentation when they make a small contribution to the file

I'm wondering if there's pre-requisite work we want to do to the template itself, and if there's any better automation tools we want to introduce first before enforcing this linter moving forwards

[gwillcox-r7]

Still pondering this PR 🤔

This might introduce a similar problem to #17582 - where it might be annoying to contributors having to update legacy documentation when they make a small contribution to the file

I'm wondering if there's pre-requisite work we want to do to the template itself, and if there's any better automation tools we want to introduce first before enforcing this linter moving forwards

I think the main goal here would be how do we reduce inconsistencies when applying our standards since this seems to be a reoccurring problem we are facing atm.

As for better automation tools if we had a way of only applying this to just the part of the code that has changed then that would be nice however I do question if this would bring additional benefit given that part of the point of this PR is to encourage us updating the docs to the newer format and making things more consistent. Ultimately happy with either way but not presently aware of any way to narrow down the scope to just changed code.

It may be an idea to front load some of this work though and put in fixes before this lands to minimize the chances of people hitting this when its not directly related to their code changes though 🤔

[smcintyre-r7]

I agree with @adfoster-r7's concerns. The existing documentation should probably all be updated to pass the lint script before we enforce it on all new submissions. Otherwise we leave contributors that copy and paste old docs wondering why it was acceptable in the existing one but not in the new one. "We're more strict now" is a conversation I'd prefer to avoid having if we can.

Do we just want to close this out for the time being?

[gwillcox-r7]

Hmm alright I'd want to check what effort would be required to change these files, but I'm happy to hold off on this PR until those changes are implemented. I'd like to try carry this across the line so if we need to change the files first I'm happy to do that, just let me know what the expected process would be and we can put this into draft or archived state until that work is done.

[github-actions[bot]]

Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it attic and closed it for now.

What does this generally mean? It could be one or more of several things:

• It doesn't look like there has been any activity on this pull request in a while
• We may not have the proper access or equipment to test this pull request, or the contributor doesn't have time to work on it right now.
• Sometimes the implementation isn't quite right and a different approach is necessary.

We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this!