-
Notifications
You must be signed in to change notification settings - Fork 129
feat: add topic and sub-command info to help output #798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add topic and sub-command info to help output #798
Conversation
|
| 'create an invite from prompted input for the specified channel', | ||
| ], | ||
| ]) | ||
| .epilog(buildEpilog({ command })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an example of a command that doesn't strictly need an update. Since it's a leaf command and doesn't use any notes or have any API docs, this effectively does nothing. (The buildEpilog function in this case returns an empty string which yargs is happy to ignore, or maybe it appends it, either having the same effect. 😄)
I'm kind of leaning towards including this in even these cases but I could be convinced otherwise. The main reason to add it would be that if we don't and a command gets sub-commands added, we will have to remember to update the parent command with an epilog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with including it even in cases like this, mostly for the reason you mentioned but also to keep everything consistent.
|
|
||
| it('returns direct, and only direct, sub-commands and topics for devices', () => { | ||
| const result = findTopicsAndSubcommands('devices') | ||
| expect(result.topics).toStrictEqual(['devices::history']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this line useful given the line below that tests for strictEqual of the whole thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not. That was leftover from when I was debugging broken tests and I forgot to remove it. Fixed now.
src/__tests__/lib/help.test.ts
Outdated
| }) | ||
|
|
||
| it('includes note from formattedNotes', () => { | ||
| expect(buildEpilog({ command: 'test', formattedNotes: 'formatted node' })).toBe('Notes:\nformatted node') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't matter, of course, but "node" or "note"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It matters to me! Fixed.
src/__tests__/lib/help.test.ts
Outdated
| findTopicsAndSubcommandsMock.mockReturnValueOnce({ topics: ['test::topic'], subCommands: [] }) | ||
|
|
||
| expect(buildEpilog({ command: 'test', formattedNotes: 'formatted node' })) | ||
| .toBe('Notes:\nformatted node\n\nTopics:\n test::topic') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note/node here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed here too.
| 'create an invite from prompted input for the specified channel', | ||
| ], | ||
| ]) | ||
| .epilog(buildEpilog({ command })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with including it even in cases like this, mostly for the reason you mentioned but also to keep everything consistent.
86e9c57 to
1e27610
Compare
First of two parts for adding topic and sub-command help to each command's help.
This PR adds:
command-utilmodule to help get topic and sub-command information for a given command.helpmodule:itemInputHelpTexthere which generates help for use with the item-input modulebuildEpilogwhich will build an epilog for use with yargs'.epilogwhich supports a variety of things we like to put in the help outputbuildEpilogThe second part of this will:
buildEpilogapi-commandmodule