Fix expected help message to show header

[madanalogy]

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X] Bug Fix

Resolves #644

What is the rationale for this request?

Improves consistency of markbind's CLI interface.

What changes did you make? (Give an overview)

I added a function to handle help messages for use with commander. The function prints the banner and is used in commander's helpOption() method.

Provide some example code that this change will affect:

function handleHelp(txt) {
  printHeader();
  logger.log('');
  return txt;
}
program
  .helpOption('-h, --help', handleHelp('output usage information'));

Is there anything you'd like reviewers to focus on?

Updating of the commander module to latest version, whether it was done properly.

Testing instructions:

From the main working directory, run node index.js -h

Proposed commit message: (wrap lines at 72 characters)

Fixes CLI help command.

[yamgent]

Hi, thank you for contributing to MarkBind! Some key notes:

• There were some problems with updating commander:

  • For future references, updating/installing a package through npm can simply be done like this: npm i commander@X.Y.Z. There should be no need to manually update package.json by hand most of the time.
  • Updating to the new commander breaks the commands on my side (e.g. I tried markbind serve and it no longer works, it just prints out the help message).
  • I think helpOption(..., helpOptionText) is meant to replace only the -h, --help <helpOptionText> part of the entire help message, rather than replacing the entire help message. The reason why calling printHeader() inside handleHelp() seems to work is because helpOption() needs to obtain the string for the initialization part. However, this means that printHeader() is always called, regardless of whether the user uses the help option or not. This is why you see duplicated headers when attempting to call other commands.

• I don't think there's a need to update commander at all in fact (I would recommend against upgrading packages unless the feature is really critical).

  • program.outputHelp() seems to be the one responsible for printing out the help message. Can you explore overriding this function to allow our header to be added?

[madanalogy]

Thank you for the information and suggestion :)

I have reverted the changes to the json files and implemented an override for outputHelp (instead of my earlier initial implementation). I will also be editing the PR title, thanks again for pointing me in the right direction!

[yamgent]

Nice, it is working better now.

Can you:

• Rebase your branch so that all of the new changes in MarkBind's master is incorporated? Your branch is slowly getting outdated with the main branch.
• Squash all your commits into just one commit "Fix expected help message to show header". Note that the git's convention is to use imperative words whenever we write the commit title/PR title (so it is "Fix" rather than "Fixes")

[yamgent]

It would be better if the comment mentions the why (the reason for overriding) as well.

You should mention that we want to customize the help message with MarkBind's header, but commander.js doesn't provide any API to customize the help message.

// We want to customize the help message to print MarkBind's header,
// but commander.js does not provide an API directly for doing so.
// Hence we override commander's outputHelp() completely.

[yamgent]

Is this line needed? I don't think anybody outside index.js uses the program variable inside here.

[yamgent]

I was wondering whether it would be even better if we just shadow the original method, and utilise it in our new method. This way, we do not have to duplicate the original method's logic at all, in case there's an update to the commander's logic when we move to a new version:

program.defaultOutputHelp = program.outputHelp;
program.outputHelp = function (cb) {
  printHeader();
  this.defaultOutputHelp(cb);
};

[madanalogy]

Thank you for the suggestions! I have changed the implementation to shadow instead of replace and updated the code comment. I have also rebased and squashed my commits.

[yamgent]

Excellent job, thanks. 👍