feat: add support for custom help classes #141

chadian · 2020-04-09T20:46:54Z

No description provided.

jgwhite · 2020-08-13T09:41:05Z

Rebased this and reformulated the commits a little bit.

jgwhite · 2020-09-08T12:38:12Z

Got the tests for this to a good place.

Co-authored-by: Chad Carbert <chadcarbert@me.com>

amphro · 2020-10-29T16:34:39Z

src/commands/readme.ts

      const plugin = new Config.Plugin({root: p, type: 'core'})
      await plugin.load()
      config.plugins.push(plugin)
    } catch {}
-    await config.runHook('init', {id: 'readme', argv: this.argv})
-    let readme = await fs.readFile('README.md', 'utf8')
+    await (config.runHook as any)('init', {id: 'readme', argv: this.argv})


Why the need for the as any?

Not sure, will investigate.

Ah yes, it’s due to this: oclif/config#158

amphro · 2020-10-29T16:37:45Z

src/commands/readme.ts

    const header = () => `## \`${config.bin} ${this.commandUsage(config, c)}\``
+
+    const generateCommandHelp = () =>  {
+      const original = process.stdout.write


I don't love this. Can we make the help class more flexible to return the command help instead? e.g help.formatCommandHelp and the showCommandHelp would call that to pass to stdout.

We can, but it would mean a breaking change to @oclif/plugin-help because we currently define the minimum contract to be just showHelp and showCommandHelp and in the docs we advise users to write directly to stdout.

The base help class already has format methods. It seems like a natural extension to make it part of the interface. It wouldn't really be a breaking change. It would just mean that if people want to take advantage of the new customer help in the readme they would have to implement the additional methods in the interface.

Also, we should still advise writing directly to stdout in the show methods, but update the docs to utilize the format methods for additional abstraction and customization.

With that said, I don't feel strongly enough to fight for it. I just don't like this pattern.

if people want to take advantage of the new customer help in the readme they would have to implement the additional methods in the interface

I think this might be the way to go. Perhaps something like:

$ npm run oclif-dev readme › Error: You are using a custom help class without required format methods. › Please define `formatCommand` etc. › Please see https://oclif.io/docs/help_classes for more.

I’ve addressed this in 57b01c9.

With the new approach (which is close in spirit to @chadian’s original approach) the readme generator will try to use formatCommand, then command, then throw a helpful error if there’s nothing usable on the custom help class.

The error looks like this:

amphro · 2020-10-29T16:38:23Z

src/commands/readme.ts

-    const help = new Help(config, {stripAnsi: true, maxWidth: columns})
+    const HelpClass = getHelpClass(config)
+    const help = new HelpClass(config, {stripAnsi: true, maxWidth: columns})
+
    const header = () => `## \`${config.bin} ${this.commandUsage(config, c)}\``


I would love for this to be configurable too via the help class or some other mechanism. That can obviously be done in another PR though.

amphro · 2020-10-29T16:39:13Z

src/commands/readme.ts

    return compact([
      header(),
-      title,
-      '```\n' + help.command(c).trim() + '\n```',
+      '```\n' + stripAnsi(generateCommandHelp().trim()) + '\n```',


We pass stripAnsi to the help class. Why do we have too do it here too?

It’s been a while since I last looked at this, but I think it’s because consumers who build upon HelpBase can do whatever they want in showHelp and showCommandHelp. stripAnsi is only used in the concrete Help implementation.

If the stripAnsi is in the concert Help, then that implies to me that custom help implementation should also strip if that option is passed in. It's not a big deal to do it here though, it just removes the flexibility if someone didn't want to strip the ansi, for whatever random reason.

amphro · 2020-10-29T17:26:57Z

src/commands/readme.ts

    return compact([
      header(),
-      title,
-      '```\n' + help.command(c).trim() + '\n```',
+      '```\n' + stripAnsi(generateCommandHelp().trim()) + '\n```',


If the stripAnsi is in the concert Help, then that implies to me that custom help implementation should also strip if that option is passed in. It's not a big deal to do it here though, it just removes the flexibility if someone didn't want to strip the ansi, for whatever random reason.

jgwhite · 2020-11-03T10:14:04Z

Going to take another run at this after discussing with @amphro.

chadian · 2020-11-10T11:07:23Z

I will be helping shepherd this PR along on

# [1.24.0](v1.23.1...v1.24.0) (2020-11-19) ### Features * add support for custom help classes ([#141](#141)) ([810e7c1](810e7c1))

oclif-bot · 2020-11-19T22:43:30Z

🎉 This PR is included in version 1.24.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

chadian force-pushed the cc/add-support-for-help-plugins branch 5 times, most recently from 0daba05 to 8e97b53 Compare April 13, 2020 17:21

chadian changed the title ~~feat: add support for help plugins~~ feat: add support for custom help classes Apr 14, 2020

RasPhilCo added this to To Do in oclif Jul 2, 2020

jgwhite mentioned this pull request Aug 3, 2020

fix(hooks): match type of Config.runHook to IConfig.runHook oclif/config#158

Closed

RasPhilCo moved this from To Do to In Progress in oclif Aug 7, 2020

jgwhite force-pushed the cc/add-support-for-help-plugins branch 2 times, most recently from c2e4af0 to 3ed0540 Compare August 13, 2020 09:40

jgwhite changed the title ~~feat: add support for custom help classes~~ feat(readme): add support for custom help classes Aug 13, 2020

jgwhite force-pushed the cc/add-support-for-help-plugins branch 5 times, most recently from b05b6bc to 1eef942 Compare September 7, 2020 17:08

jgwhite changed the title ~~feat(readme): add support for custom help classes~~ feat(readme)!: add support for custom help classes Sep 8, 2020

Jamie White and others added 6 commits September 8, 2020 15:21

Bump oclif dependency versions to void type errors

1d59953

Co-authored-by: Chad Carbert <chadcarbert@me.com>

Add temporary workaround for runHook issue

41b9eb2

Co-authored-by: Chad Carbert <chadcarbert@me.com>

Refactor run method

df42c56

Co-authored-by: Chad Carbert <chadcarbert@me.com>

Add extra checks for plugin.pjson.devDependencies

40727a3

Co-authored-by: Chad Carbert <chadcarbert@me.com>

Use custom help class to render README help

31cec2d

Co-authored-by: Chad Carbert <chadcarbert@me.com>

Add test for readme-generation with custom help

85745f2

Co-authored-by: Chad Carbert <chadcarbert@me.com>

jgwhite force-pushed the cc/add-support-for-help-plugins branch from 1eef942 to 85745f2 Compare September 8, 2020 13:22

jgwhite self-assigned this Sep 9, 2020

jgwhite marked this pull request as ready for review September 10, 2020 14:59

amphro reviewed Oct 29, 2020

View reviewed changes

amphro approved these changes Oct 29, 2020

View reviewed changes

Jamie White added 2 commits November 5, 2020 14:04

Use Help.formatCommand or Help.command or error with a nice message

57b01c9

Improve custom help error output

c99cff1

jgwhite assigned chadian and unassigned jgwhite Nov 10, 2020

RasPhilCo and others added 2 commits November 19, 2020 13:28

Merge branch 'master' into cc/add-support-for-help-plugins

a6d667d

yarn.lock

ac51778

RasPhilCo changed the title ~~feat(readme)!: add support for custom help classes~~ feat: add support for custom help classes Nov 19, 2020

RasPhilCo merged commit 810e7c1 into master Nov 19, 2020

RasPhilCo deleted the cc/add-support-for-help-plugins branch November 19, 2020 21:56

oclif-bot added a commit that referenced this pull request Nov 19, 2020

chore(release): 1.24.0 [skip ci]

7386f79

# [1.24.0](v1.23.1...v1.24.0) (2020-11-19) ### Features * add support for custom help classes ([#141](#141)) ([810e7c1](810e7c1))

oclif-bot added the released label Nov 19, 2020

This was referenced Nov 20, 2020

Regression: readme no longer shows command title #267

Closed

fix: restore command descriptions in README generation #268

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for custom help classes #141

feat: add support for custom help classes #141

chadian commented Apr 9, 2020 •

edited by jgwhite

jgwhite commented Aug 13, 2020

jgwhite commented Sep 8, 2020

amphro Oct 29, 2020

jgwhite Oct 29, 2020

jgwhite Oct 29, 2020

amphro Oct 29, 2020

amphro Oct 29, 2020

jgwhite Oct 29, 2020

amphro Oct 29, 2020

jgwhite Nov 2, 2020

jgwhite Nov 5, 2020

amphro Oct 29, 2020

amphro Oct 29, 2020

jgwhite Oct 29, 2020

amphro Oct 29, 2020

amphro Oct 29, 2020

jgwhite commented Nov 3, 2020

chadian commented Nov 10, 2020

oclif-bot commented Nov 19, 2020

feat: add support for custom help classes #141

feat: add support for custom help classes #141

Conversation

chadian commented Apr 9, 2020 • edited by jgwhite

jgwhite commented Aug 13, 2020

jgwhite commented Sep 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgwhite commented Nov 3, 2020

chadian commented Nov 10, 2020

oclif-bot commented Nov 19, 2020

chadian commented Apr 9, 2020 •

edited by jgwhite