Fix hidden commands changing padding in usage template

[aliscott]

We added a fig-autocomplete hidden subcommand to Infracost and it increased the padding in the usage text (https://github.com/infracost/infracost/pull/1457#discussion_r827182625), which was unexpected. This fixes any hidden commands affecting the padding:

Fixes #1272

Example before:

AVAILABLE COMMANDS
  breakdown        Show full breakdown of costs
  comment          Post an Infracost comment to GitHub, GitLab, Azure Repos or Bitbucket
  completion       Generate shell completion script

Example with fix:

AVAILABLE COMMANDS
  breakdown   Show full breakdown of costs
  comment     Post an Infracost comment to GitHub, GitLab, Azure Repos or Bitbucket
  completion  Generate shell completion script

I wasn't sure where to add tests for this since I couldn't find any existing ones for this functionality, but if you point me in the right direction I can do.

[CLAassistant]

All committers have signed the CLA.

[marckhouzam]

This makes sense. I'm surprised it wasn't fixed earlier 😄 . Thanks for the PR! In fact I think the same problem applies to "deprecated" commands. I think that instead of checking for Hidden, we could instead use IsAvailableCommand() to know if the command should be considered for padding.

As for tests, I didn't find any related to padding (but I may have missed them), but you can add them in the command_test.go file.

[aliscott]

Thanks @marckhouzam have updated to use IsAvailableCommand() and added some tests.

[marckhouzam]

I think IsAvailableCommand() only works when the command has subcommands or has a run function. I think it is possible to have doc only commands which do not have a run function so this would break

Good point @Luap99. I hadn't thought of that.

I haven't used an AdditionalHelpTopic before (what you call "doc only command"), do you have a use case that would help understand when to use such a thing?

[marckhouzam]

@aliscott As pointed out by @Luap99, we should consider the case of AdditionalHelpTopic, so I think we should check that the command is neither hidden nor deprecated explicitly instead of checking IsAvailableCommand().

[Luap99]

I don't use it and I am not aware of cobra examples but one simple example is go help, e.g. go help go.mod

[Luap99]

Also see the linked issue in the function comment: https://github.com/spf13/cobra/issues/393#issuecomment-282741924

[marckhouzam]

Also see the linked issue in the function comment: #393 (comment)

Brilliant! I had not noticed the link in the comment 🤦‍♂️

[aliscott]

Thanks @marckhouzam and @Luap99. Updated to use .Hidden and .Deprecation, and rebased to solve the conflicts.

[marckhouzam]

@jpmcb This is ready for a second review.

[marckhouzam]

This looks good to me - but I'm a bit confused as to the root cause of this. Why are commands that are deprecated or hidden getting different paddings?

The padding, as far as I know is used when printing the help text. Hidden and deprecated commands are not shown in the help text so they should not be used when calculating the padding; you could have a much longer hidden command which would cause a lot of padding, when the help won't even show that command.

But your question is making me wonder if someone overrides the help text and chooses the display deprecated commands for example, would we be causing them an issue?

[jpmcb]

you could have a much longer hidden command which would cause a lot of padding, when the help won't even show that command.

Ahhh that makes sense.

wonder if someone overrides the help text and chooses the display deprecated commands for example, would we be causing them an issue?

Hmmmm good question. We should investigate a bit further to confirm we won't break users

[github-actions[bot]]

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed. You can:
• Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening.

[jpmcb]

Not stale - needs more discussion

[github-actions[bot]]

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed. You can:
• Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening.

[marckhouzam]

I put the lifecycle/frozen label on this one.