Skip to content
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

Expand help customisation with .addHelpText #1296

Merged
merged 39 commits into from
Sep 9, 2020

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Jul 4, 2020

Pull Request

Problem

  1. can not add a splash screen/title/banner to the help
  2. can not add a global epilog
  3. event listener is a little clunky and unfamiliar to use
  4. help is displayed on stdout even when being displayed as an "error"

See #1225 for collected custom help issues, and #997 for error output.

Solution

Add .addHelpText() which takes a position as first parameter, and a string to add or a function returning a string. Position is 'before' or 'after' to affect just this command, and 'beforeAll' or 'afterAll' to affect this command and all its subcommands.

For example:

program.addHelpText('afterAll', '/nSee the web site for more information');

Also:

  • .help() and .outputHelp() can be passed { error: true } to write to stderr.
  • Deprecate passing callback to .help() and .outputHelp()
  • Deprecate .on('--help')

ChangeLog

  • use .addHelpText() to add text before or after the built-in help, for just current command or also for all subcommands (Expand help customisation with .addHelpText #1296)
  • deprecated callback parameter to .help() and .outputHelp() (removed from README)
  • deprecate .on('--help') (removed from README)

@shadowspawn shadowspawn marked this pull request as draft July 4, 2020 06:56
@westhom

This comment has been minimized.

@shadowspawn

This comment has been minimized.

@westhom

This comment has been minimized.

@shadowspawn shadowspawn added the semver: major Releasing requires a major version bump, not backwards compatible label Aug 2, 2020
@shadowspawn
Copy link
Collaborator Author

To be safe, the change to use stderr for displaying the help in response to an error makes it a semver major change.

The other changes are backwards compatible, as the --help event and the .help(cb) callback are deprecated but not removed.

@shadowspawn shadowspawn changed the title WIP: Expand custom help features Expand custom help features with more events Aug 8, 2020
@shadowspawn

This comment has been minimized.

@shadowspawn shadowspawn changed the title WIP: Expand help customisation with .addHelpText Expand help customisation with .addHelpText Sep 4, 2020
@shadowspawn shadowspawn marked this pull request as ready for review September 4, 2020 22:06
@shadowspawn
Copy link
Collaborator Author

I am happier with how this is looking now with an explicit routine which is more readable than an event listener, and leaving the ability to override the built-in help out of this routine.

@shadowspawn shadowspawn removed their assignment Sep 6, 2020
@shadowspawn
Copy link
Collaborator Author

I deprecated the callbacks to .help() and .outputHelp() as this PR originally included being able to override the built-in help too. That is no longer included, but I would still like to call the callbacks deprecated so can remove them in future!

@abetomo
Copy link
Collaborator

abetomo commented Sep 9, 2020

Could you merge develop into release/7.x?
This includes the 6.1.0 differences.

@shadowspawn
Copy link
Collaborator Author

Oops, will do. I was merging 6.1 into the PR so the diff against develop looked better, missed doing 7.x too.

@shadowspawn
Copy link
Collaborator Author

I have merged develop into release/7.x.

Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Sep 9, 2020
@shadowspawn shadowspawn added this to the v7.0.0 milestone Sep 9, 2020
@shadowspawn shadowspawn merged commit d26a26d into tj:release/7.x Sep 9, 2020
@shadowspawn shadowspawn deleted the feature/on-help branch September 13, 2020 07:30
@shadowspawn shadowspawn mentioned this pull request Oct 14, 2020
@crystalfp
Copy link

Only a (minor) issue in the new version:

  1. .addHelpText("after", "\n") adds two blank lines after help.
  2. .addHelpText("after", "") adds nothing after help.
  3. .addHelpText("after", " ") adds one blank line after help.

A little confusing. If case 1 should behave as console.log() then case 2 should add one blank line after help. Instead, if the function behaves like write() then case 1 should add one blank line, in my opinion.

Besides this minor issue, the new version is excellent. Thanks!
mario

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Jan 15, 2021

Case 2 is a special case. If there is an empty string then nothing is output. This is to support a function generating the string deciding that there is nothing to be displayed.

@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jan 16, 2021
@crystalfp
Copy link

OK, understand. Maybe adding a line to documentation stating this could be useful. Something like this at the end of the Custom help paragraph: “If the string or the output of the function is an empty string, nothing is output, otherwise the string is output followed by a newline”.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: major Releasing requires a major version bump, not backwards compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants