Fix help text indentation

[ybiquitous]

Because of TAB included in some lines, help text indentation is broken. This fixes the bug.

Two commits included:

1. Check the current help text on system tests.
2. Fix the help text.

Please compare the diff between each commit.

[hudochenkov]

I would say having expectedHelp.txt is inconvenient because as soon we change text in a code we have to update the test. It makes that test a chore and don't really help.

Can we just update --help test so it verify that no tabs present in a help text?

[ybiquitous]

@hudochenkov I understand what you say, but I'd like to test a broken indentation, not to test including TABs.

Also, I have the following reasons to test the help text:

• I'd like to prevent an unexpected regression when we refactor cli.js. I have a plan to refactor cli.js for better type-checking on other PRs.
• I think that the chore task updating expectedHelp.txt will rarely occur. Because we have changed rarely the help text content itself.

What do you think? I'm glad to accept if there is a better way! 😄

[ntwb]

How about using the new Jest "inline snapsot" functionality instead?

https://jestjs.io/docs/en/snapshot-testing#inline-snapshots
"Inline snapshots behave identically to external snapshots (.snap files), except the snapshot values are written automatically back into the source code. This means you can get the benefits of automatically generated snapshots without having to switch to an external file to make sure the correct value was written."

[ybiquitous]

@ntwb Thanks, looks good. But I'm unfamiliar with the functionality. If you can change soon, welcome to push your change to this PR! 🙌

[ybiquitous]

I pushed e9ce817 using .toMatchInlineSnapshot(). Thank you @ntwb!

@hudochenkov How about this?

[vankop]

Looks like we need ability to generate help text =)

@ybiquitous Unfortunately this test does not make much sense, since it is just a copy paste.

From my perspective we need move each option description in meow settings aka:

const options = {
    'formatter': {
          description: `allowed formatters ${getFormatterOptionsText({ useOr: true })}`,
          default: 'string'
    },
    'no-color': {
         description: 'blabla'
    }
}

options.help = generateHelp(options);

[hudochenkov]

.toMatchInlineSnapshot() is better, but it takes too much space in test file. Maybe we should use regular snapshots instead (toMatchSnapshot)?

[ybiquitous]

@vankop Thanks for your feedback. The snapshot value is automatically generated when we run the test, so I don't think it's just a copy-paste.

generateHelp() sounds like an attractive idea to me but I think the idea is out of the scope of this PR.

[ybiquitous]

@hudochenkov I can accept also toMatchSnapshot(), but don't we give up the auto-generation of a snapshot value?

[hudochenkov]

@ybiquitous regular snapshot are generated by Jest. If snapshot doesn't match, we can tell Jest to update it. Shapshots are not supposed to be changed manually.

[ybiquitous]

Make sense! 👍 I will fix to use toMatchSnapshot() soon.

[hudochenkov]

Thanks!

[ybiquitous]

@hudochenkov Thanks, too. After all approval, I will squash to one commit before merge.

[hudochenkov]

No need for squashing, I'll do it on merge with a press of a single button :)

[ntwb]

I was also thinking along the lines of what @vankop wrote above in #4531 (comment)

Instead of testing for the entire output, each cli option could have it's own test to ensure the output of each command would match the test, this could use either toMatchSnapshot() or toMatchInlineSnapshot() and I suggested the inline snapshot as it saves and regenerates inline in the source file rather than another file.

Anyways, not a blocker for this PR, happy to see it merged and this could be considered in a follow up PR. It maybe beneficial to do something like this to improve the cli tests before adding types.

[ybiquitous]

@ntwb Thanks for your reply.

Sadly, meow does not have the auto-generation of help text:
sindresorhus/meow#80

So, we have to write our own code or contribute to meow. There may be based implementation already: sindresorhus/meow#82

I suggested the inline snapshot as it saves and regenerates inline in the source file rather than another file.

If we test each description instead of the whole help text, the inline snapshot may be better. 👍

It maybe beneficial to do something like this to improve the cli tests before adding types.

I completely agree. 😃

[vankop]

I suggested the inline snapshot as it saves and regenerates inline in the source file rather than another file.

Damn it behave so unobviously =( I thought that @ybiquitous copy paste help info 🤦‍♂️ Maybe better to keep snapshots as separated files