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

WIP: Refactor help internals #1346

Closed

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Sep 6, 2020

Pull Request

Inspired by #1338, trying out a different approach.

Problem

The internal help routines have a lot of logic scattered around, and are not accessible for custom help implementations.

Solution

Use some consistent routines for commands, options, and arguments.

@shadowspawn shadowspawn self-assigned this Sep 6, 2020
@cravler
Copy link
Contributor

cravler commented Sep 9, 2020

Hi @shadowspawn

Looks nice, but maybe we need implement Help class?

const { Command } = require('./index');

/* WIP */
class Help {

  constructor(cmd) {
    this.cmd = cmd;
  };

  usage() {
    return this.cmd.fullName() + ' ' + this.cmd.usage();
  };

  description() {
    return this.cmd.description();
  };

  arguments() {
    return this.cmd.visibleArguments();
  };

  options() {
    return this.cmd.visibleOptions().map(option => ({term: option.flags, description: option.fullDescription()}));
  };

  commands() {
    return this.cmd.visibleCommands().map(cmd => ({term: cmd.helpTerm(), description: cmd.description()}));
  };

  largestTermLength() {
    return Math.max(
      this.cmd.largestArgLength(),
      this.cmd.largestOptionLength(),
      this.cmd.largestCommandHelpTermLength()
    );
  };

  pad(str, width) {
    return pad(str, width);
  };

  wrap(str, width, indent) {
    return optionalWrap(str, width, indent);
  };

  render() {
    const width = this.largestTermLength();
    const columns = process.stdout.columns || 80;
    const descriptionWidth = columns - width - 4;

    const formatList = (arr) => {
      return arr.map(({term, description}) => {
        if (description) {
          return this.pad(term, width) + '  ' + this.wrap(description, descriptionWidth, width + 2);
        }
        return term;
      }).join('\n').replace(/^/gm, '  ');
    }

    // Usage
    let output = ['Usage: ' + this.usage(), ''];

    // Description
    if (this.description()) {
      output = output.concat([this.description(), '']);
    }

    // Arguments
    if (this.arguments().length) {
      output = output.concat(['Arguments:', formatList(this.arguments()), '']);
    }

    // Options
    if (this.options().length) {
      output = output.concat(['Options:', formatList(this.options()), '']);
    }

    // Commands
    if (this.commands().length) {
      output = output.concat(['Commands:', formatList(this.commands()), '']);
    }

    return output.join('\n');
  };
}

class MyHelp extends Help {
  // custom render method
}

class MyCommand extends Command {
  createCommand(name) {
    return new this.constructor(name);
  };

  /* WIP */
  fullName() {
    let cmdName = this.name();
    if (this.alias()) {
      cmdName = cmdName + '|' + this.alias();
    }
    let parentCmdNames = '';
    for (let parentCmd = this.parent; parentCmd; parentCmd = parentCmd.parent) {
      parentCmdNames = parentCmd.name() + ' ' + parentCmdNames;
    }
    return parentCmdNames + cmdName;
  };

  /* WIP */
  helpInformation() {
    return (new MyHelp(this)).render();
  };
}

@shadowspawn
Copy link
Collaborator Author

The examples we have been writing are very interesting, but my current thinking is I won't be adding major classes or structures now. I don't have a good enough idea of how to make it easy for people to do custom help. Subclassing is an approach but I think is quite heavy weight. I am also thinking about wider approaches to make it easier to "plugin" custom behaviours, with help as an example, and middleware as another.

I am likely to make smaller changes for now. A specific concern is making pad and wrap accessible in some way which is a bit of a blocker to custom help currently. (As you were aware too, adding them to Help class.)

@cravler
Copy link
Contributor

cravler commented Sep 14, 2020

About pad and wrap, would be good to have something like:

const { pad, wrap, optionalWrap } = require('commander/util');

@shadowspawn
Copy link
Collaborator Author

A separate Help class is good for encapsulation, but seems clumsy for subclassing as need to subclass Command and Help, and tie them together somehow.

I wonder (but have not experimented) whether a hash of helper routines would provide some namespace separation but still be easy to override in a subclass of Command, or supply overrides.

const widest = program.helpUtils.largestTermLength();

See for example fairly limited but lots of hooks: https://sywac.io/docs/style-hooks.html

@shadowspawn
Copy link
Collaborator Author

I personally like subclass from experience with other languages, but feel it is not the most flexible Javascript way. Having said that, I am happy with how createCommand has worked out so far. I might have a play with createHelp as the hook for the suggested Help class/interface.

@shadowspawn
Copy link
Collaborator Author

Closing this in favour of #1365

@shadowspawn shadowspawn closed this Oct 2, 2020
@shadowspawn shadowspawn deleted the feature/refactor-help=internals branch October 25, 2020 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants