Skip to content

feat: creating custom help string#103

Merged
Souvikns merged 10 commits intoasyncapi:masterfrom
Souvikns:new-help-string
Nov 3, 2021
Merged

feat: creating custom help string#103
Souvikns merged 10 commits intoasyncapi:masterfrom
Souvikns:new-help-string

Conversation

@Souvikns
Copy link
Member

Description
With the migration to OCLIF, our previous help has been updated, so this PR aims to restore the help string as before,

Though functionally the OCLIF's help string out of the box is very helpful, So should we update this, or use what OCLIF supports out of the box. Maybe we could have some color-coding different sections. Any opinion @derberg @fmvilas @magicmatatjahu

Oclif's help strings:

All in one CLI for all AsyncAPI tools

VERSION
  @asyncapi/cli/0.6.3 linux-x64 node-v16.13.0

USAGE
  $ asyncapi [COMMAND]

TOPICS
  config  access configs

COMMANDS
  config    access configs
  help      display help for asyncapi
  validate  validate asyncapi file

@fmvilas
Copy link
Member

fmvilas commented Oct 28, 2021

I honestly like the default Oclif help message. Looks like the default for all CLIs out there.

@derberg
Copy link
Member

derberg commented Oct 28, 2021

yyy, is it just me or do others also have no clue what Topics are about 😄

this is what we had:
Screenshot 2021-10-28 at 11 12 27

I heading look pretty standard to me, maybe we need to change Options to Flags as in GH client:
Screenshot 2021-10-28 at 11 14 38


Anyway, we switched to oclify to get some defaults in place and speed up. So let us use oclify for help too. I mean the most important is just to stick to one way of doing things, and better to use what is there already and not reinvent our naming. So let us just use what even oclify CLI does:

Screenshot 2021-10-28 at 11 18 16

So options turn into flags, and commands into arguments, and when there is description provided for a given command, it goes under description

@Souvikns
Copy link
Member Author

yyy, is it just me or do others also have no clue what Topics are about 😀

I was also struggling to understand the concept of topics, but reading their docs, topics are like nested commands
so asyncapi config context list both config and context is a topic that signifies that they have nested commands

so in our root help even though we have 3 (help command has to be removed I guess ) commands only config is added into TOPIC. If anyone knows about what topic is then it is really helpful, I guess we can talk about topics in our documentation.

TOPICS
  config  access configs

COMMANDS
  config    access configs
  help      display help for asyncapi
  validate  validate asyncapi file

and commands into arguments

I think changing commands into arguments would be confusing, consider the validate help string

validate asyncapi file

USAGE
  $ asyncapi validate [SPEC-FILE]

ARGUMENTS
  SPEC-FILE  spec path or context-name

OPTIONS
  -h, --help  show CLI help

In asyncapi validate [SPEC-FILE] validate is a command if we call it an argument then what will SPEC-FILE be called.

@fmvilas
Copy link
Member

fmvilas commented Oct 28, 2021

Yeah, Topics should be removed. They don't make sense anymore since we're not using the default Oclif notation which is something like asyncapi config:context:add .... We have this overridden with a plugin so things are separated by a space, not a colon: asyncapi config context add .... Therefore topics are not needed anymore.

Also, agree with Lukasz that options must be called flags. I think it's more common. Actually, in Oclif code, they're called flags too 😄

@Souvikns
Copy link
Member Author

Souvikns commented Oct 28, 2021

The plugin was fairly easy to change, only they didn't provide any way to change FLAGS from OPTIONS. So I had to copy, the underlying logic for the test formating for consistency.

Screenshot from 2021-10-28 17-48-04

@Souvikns Souvikns marked this pull request as ready for review October 28, 2021 12:21
@@ -0,0 +1,161 @@
import * as Config from '@oclif/config'
Copy link
Member

Choose a reason for hiding this comment

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

You got a few things to fix in this file 😅

@Souvikns
Copy link
Member Author

Souvikns commented Nov 1, 2021

ready to be reviewed 🤞🏻

fmvilas
fmvilas previously approved these changes Nov 3, 2021
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

:shipit:

@fmvilas
Copy link
Member

fmvilas commented Nov 3, 2021

@Souvikns you need to resolve some conflicts with the master branch but other than that it looks great to me 🙌

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Souvikns
Copy link
Member Author

Souvikns commented Nov 3, 2021

@fmvilas I think I need another approval.

@Souvikns Souvikns merged commit 6099147 into asyncapi:master Nov 3, 2021
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@fmvilas
Copy link
Member

fmvilas commented Nov 3, 2021

@Souvikns CLI is broken again 😅 I get this error this time:

$ asyncapi --help
    Error: Unable to load configured help class "./src/help/index.ts", failed with message:
    Cannot find module '/Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/src/help/index.ts'
    Require stack:
    - /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/node_modules/@oclif/plugin-help/lib/u
    til.js
    - /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/node_modules/@oclif/plugin-help/lib/c
    ommand.js
    - /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/node_modules/@oclif/plugin-help/lib/i
    ndex.js
    - /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/node_modules/@oclif/command/lib/comma
    nd.js
    - /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/node_modules/@oclif/command/lib/index
    .js
    - /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/@asyncapi/cli/bin/run

@fmvilas
Copy link
Member

fmvilas commented Nov 3, 2021

I think you have to change this line to:

"helpClass": "./lib/help"

This is what I see when I look into the installed module:

$ ll /Users/fmvilas/.nvm/versions/node/v14.15.1/lib/node_modules/\@asyncapi/cli/lib/help
total 32
drwxr-xr-x   6 fmvilas  staff   192B  3 nov 08:48 .
drwxr-xr-x  10 fmvilas  staff   320B  3 nov 08:48 ..
-rw-r--r--   1 fmvilas  staff   219B 26 oct  1985 command.d.ts
-rw-r--r--   1 fmvilas  staff   2,2K 26 oct  1985 command.js
-rw-r--r--   1 fmvilas  staff   305B 26 oct  1985 index.d.ts
-rw-r--r--   1 fmvilas  staff   1,8K 26 oct  1985 index.js

fmvilas added a commit to fmvilas/cli that referenced this pull request Nov 3, 2021
fmvilas added a commit that referenced this pull request Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants