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

Generate help by flag descriptions #140

Closed
wants to merge 15 commits into from

Conversation

ybiquitous
Copy link
Contributor

@ybiquitous ybiquitous commented Mar 22, 2020

This fixes #80. Rechallenge of PR #82.

Example

  A useful command

  Options:
    --foo <string>

    --output <string>

    --input <string>  (default: stdin)

    -i <number>, --indent <number>  (default: 2)
      Indent level

    --enabled  (default: false)
      Enabled or not

    --lw <string>, --long-word <string>  (default: none)
      Very very long long word.
      This is the second line.

Welcome any feedback!

@ybiquitous ybiquitous marked this pull request as ready for review March 22, 2020 13:53
test.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

I don't think helpOptions is a good option name. I think we should just automatically show the text if the help option (or argument) is not specified.

@sindresorhus
Copy link
Owner

sindresorhus commented Apr 7, 2020

Also look at how https://github.com/yargs/yargs solves this and how it presents the output. Also do some research on how other arguments parsers in Python and Ruby handles the config and output.

index.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

There should probably also be a method to get the autogenerated help text in case you want to manually combine it with your own help text.

@ybiquitous
Copy link
Contributor Author

Sorry for my late reply. I will work on your review this weekend.

@ybiquitous ybiquitous marked this pull request as draft April 12, 2020 15:14
@ybiquitous
Copy link
Contributor Author

ybiquitous commented Apr 12, 2020

I looked at some command-line helpers and some command-line tools. For example:

As a result, I think the following format is better:

An awesome CLI tool

Usage: foo [options]

Options:
  --input <string>            Input source. [default: stdin]
  -o, --output <string>       Output file to be reported.
  -i, --indent <number>       Indent level. [default: 2]
  --quiet                     Suppress any output.
  --verbose                   Show verbose output.
                              See also --quiet.
  -h, --help                  Show help.
  • An option's name and description on a line.
  • First short-flag, last long-flag (all tools except yargs support this order).

What do you think?

@sindresorhus
Copy link
Owner

I agree with everything except the shortflag order. The shortflag should be last.

@sindresorhus
Copy link
Owner

Indent level. [default: 2]

Do any of the libraries put more than one space before [default? It feels a bit cramped into the description.

@sindresorhus
Copy link
Owner

Another good library for inspiration: https://github.com/apple/swift-argument-parser

@ybiquitous
Copy link
Contributor Author

I agree with everything except the shortflag order. The shortflag should be last.

OK, I'll do it. 👍

Do any of the libraries put more than one space before [default? It feels a bit cramped into the description.

I have no strong opinion about it. How many spaces are appropriate?

@sindresorhus
Copy link
Owner

2

@ybiquitous
Copy link
Contributor Author

Sorry for the late response... I've addressed all the review points except for the following one:

There should probably also be a method to get the autogenerated help text in case you want to manually combine it with your own help text.

About the comment above, what kind of API do you expect?

@ybiquitous ybiquitous marked this pull request as ready for review May 18, 2020 05:31
generate-help.js Outdated
return lines;
}

module.exports = function ({description, help, flags}, pkg) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
module.exports = function ({description, help, flags}, pkg) {
module.exports = ({description, help, flags}, pkg) => {

generate-help.js Outdated

lines = lines.map(line => trimNewlines(line));

const content = lines.join(EOL).replace(/^\t+/gm, '').replace(/[\t ]+[\r\n]*$/gm, '');
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of the regexes here, couldn't you use redent?

generate-help.js Outdated

const maxNameLengh = Math.max(...entries.map(({name}) => name.length));

const lines = entries.map(({name, desc}) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Don't use abbreviations for variable/parameter names.

generate-help.js Outdated
lines = lines.map(line => trimNewlines(line));

const content = lines.join(EOL).replace(/^\t+/gm, '').replace(/[\t ]+[\r\n]*$/gm, '');
return EOL + trimNewlines(redent(content, 2)) + EOL;
Copy link
Owner

Choose a reason for hiding this comment

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

Use \n instead of EOL

@sindresorhus
Copy link
Owner

About the comment above, what kind of API do you expect?

Any suggestions?

@ybiquitous
Copy link
Contributor Author

About the comment above, what kind of API do you expect?

Any suggestions?

For example:

const { generateHelp } = require("meow");
generateHelp({ help: "...", description: "...", flags: {...} }, packageJson);

@ybiquitous
Copy link
Contributor Author

Edited:

const { generateHelp } = require("meow");
generateHelp({ help: "...", description: "...", defaultDescription: "...", flags: {...} });

@sindresorhus
Copy link
Owner

Thinking more about it, maybe instead of a free method, we could make the help option also accept a function, which would receive the full generated help text, generated each section (for example, the rendered help for flags, and other sections), and also the meow options. That should be flexible enough for the user to do anything they want. The function would be expected to return the final help text. Thoughts?

@ybiquitous
Copy link
Contributor Author

Thanks for your advice.

make the help option also accept a function

It seems a better API than my suggestion! 👍
Do you expect the following use case, for example?

const cli = meow({
  help: (wholeText: string, flagsSection: string, description: string, options: meow.Options) => {
    return `...`;
  },
  flags: {
    input: {
      type: 'string',
      description: 'Input file path'
    }
  }
});

@sindresorhus
Copy link
Owner

Yes, but I think it’s better to make help accept a single object as argument instead of lots of arguments. Since many of them will probably not be used.

@ybiquitous
Copy link
Contributor Author

Make sense. I'll try it, thanks!

@ybiquitous
Copy link
Contributor Author

The help option can receive a function via 935911f.

@ybiquitous
Copy link
Contributor Author

I'm ready to be reviewed again.

index.d.ts Outdated
/**
Callback function to generate a help text you want.
*/
type GenerateHelp = (args: Readonly<{wholeText: string; flagLines: readonly string[]; description: string; options: Readonly<Options<AnyFlags>>}>) => string;
Copy link
Owner

Choose a reason for hiding this comment

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

The args type should be a separate interface and each property should have a doc-comment.

Copy link
Owner

Choose a reason for hiding this comment

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

args should have a better name too.

index.d.ts Outdated
@@ -27,6 +28,11 @@ declare namespace meow {

type AnyFlags = {[key: string]: StringFlag | BooleanFlag | NumberFlag};

/**
Callback function to generate a help text you want.
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be more verbose. Document the default behavior, etc.

readme.md Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Some things to keep in mind:

  • Review your own diff.
  • Try to refactor and simplify the implementation.
  • Try to add more tests.
  • More docs.

@sindresorhus
Copy link
Owner

Bump

@ybiquitous
Copy link
Contributor Author

Sorry for the late response.

  • Try to refactor and simplify the implementation.
  • Try to add more tests.

I have no good ideas about the points above. Could you please help me?

@@ -0,0 +1,103 @@
'use strict';
Copy link
Owner

Choose a reason for hiding this comment

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

This file needs to be added to files in package.json.

function buildFlagLines(flags) {
flags = {...flags, help: {type: 'boolean', description: 'Show help'}};

const entries = Object.entries(decamelizeKeys(flags, '-')).map(([name, def]) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Don't use abbreviations (regarding def, and other cases).

}

/**
Callback function to customize a help text you want.
Copy link
Owner

Choose a reason for hiding this comment

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

Typos


The input is reindented and starting/ending newlines are trimmed which means you can use a [template literal](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/template_strings) without having to care about using the correct amount of indent.

The description will be shown above your help text automatically.

Also, you can customize the auto-generated help text by giving the function as follows:
Copy link
Owner

Choose a reason for hiding this comment

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

The readme and index.d.ts should be in sync.

@sindresorhus
Copy link
Owner

Try to refactor and simplify the implementation.

The code is quite hard to follow. Some code comments could maybe help that too. I often comment "simplify" on PRs as honestly most PRs could be simplified. Nobody writes perfect code the first time around. It helps to come back to it and look it over and try to simplify/refactor it. That's what I'm asking.

Try to add more tests.

I'm just asking you to look over and try to add some more tests. I'm the one that will be stuck maintaining this for infinity, so I want to ensure it's as solid as possible.

@@ -28,6 +29,37 @@ declare namespace meow {
type AnyFlag = StringFlag | BooleanFlag | NumberFlag;
type AnyFlags = {[key: string]: AnyFlag};

interface GenerateHelpOptions {
/**
A whole help text generated by default.
Copy link
Owner

Choose a reason for hiding this comment

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

Typo

description: string;

/**
A list of a line including each flag's name and description.
Copy link
Owner

Choose a reason for hiding this comment

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

Typo

flagLines: readonly string[];

/**
An object including each flag information.
Copy link
Owner

Choose a reason for hiding this comment

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

Not clear to the user what "flag information" means.

@sindresorhus
Copy link
Owner

This change needs more docs. There's not even a mention of the new behavior of generating flag docs by default.

Base automatically changed from master to main January 23, 2021 17:36
@armano2
Copy link

armano2 commented Feb 28, 2021

as for yargs, this is not fully true as they print in with a little better indentations

example:

@commitlint/[email protected] - Lint your commit messages

[input] reads from stdin if --edit, --env, --from and --to are omitted

Options:
  -c, --color          toggle colored output           [boolean] [default: true]
  -g, --config         path to the config file                          [string]
      --print-config   print resolved config          [boolean] [default: false]
  -d, --cwd            directory to execute in
                                         [string] [default: (Working Directory)]
      --edit           read last commit message from the specified file or
                       fallbacks to ./.git/COMMIT_EDITMSG               [string]
  -E, --env            check message in the file at path given by environment
                       variable value                                   [string]
  -x, --extends        array of shareable configurations to extend       [array]
  -H, --help-url       help url in error message                        [string]
  -f, --from           lower end of the commit range to lint; applies if
                       edit=false                                       [string]
  -o, --format         output format of the results                     [string]
  -p, --parser-preset  configuration preset to use for
                       conventional-commits-parser                      [string]
  -q, --quiet          toggle console output          [boolean] [default: false]
  -t, --to             upper end of the commit range to lint; applies if
                       edit=false                                       [string]
  -V, --verbose        enable verbose output for reports without problems
                                                                       [boolean]
  -v, --version        display version information                     [boolean]
  -h, --help           Show help                                       [boolean]

@ybiquitous
Copy link
Contributor Author

I am very sorry for my too late response. I can no longer go any further with this PR due to my lack, in spite of many advice I have received. 🙇

I am so grateful for the kind review.
And, I hope this PR will be of some help when someone works on implementing this feature again. ☺️

Thank you.

@ybiquitous ybiquitous closed this Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autogenerate flag help when no helpMessage is given
3 participants