Skip to content

fix: failed command usage string is missing arg descriptions and optional args#2105

Merged
bcoe merged 4 commits intoyargs:mainfrom
jly36963:fix-failed-default-command-usage-string
Jan 9, 2022
Merged

fix: failed command usage string is missing arg descriptions and optional args#2105
bcoe merged 4 commits intoyargs:mainfrom
jly36963:fix-failed-default-command-usage-string

Conversation

@jly36963
Copy link
Copy Markdown
Contributor

@jly36963 jly36963 commented Dec 27, 2021 •
edited
Loading

Addresses:
#2030
#2102

Problem

The usage from a failed default command is missing the following:

  • argument descriptions
  • optional arguments
  • possibly more (?)

Example

In the code and output below, you will see that the failed default command is missing descriptions and optional arguments. (I omitted the command usage output, as it's not part of the issue.)

yargs('')
  .command({
    command: '$0 <arg1>',
    desc: 'default desc',
    builder: yargs => yargs
      .option('arg1', {
        type: 'string',
        desc: 'arg1 desc (default command)',
        demandOption: true
      })
      .option('arg2', {
        type: 'string',
        desc: 'arg2 desc (default command)'
      }),
    handler: argv => {
      console.log(argv)
    }
  })
  .command({
    command: 'cmd1 <arg1>',
    desc: 'cmd1 desc',
    builder: yargs => yargs
      .option('arg1', {
        type: 'string',
        desc: 'arg1 desc',
        demandOption: true
      }),
    handler: argv => {
      console.log(argv)
    }
  })
  .strict()
  .parse()


/*

# cmd1
# yargs('cmd1')

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]
  --arg1     arg1 desc                                       [string] [required]

# default (failed command)
# yargs('')

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]
  --arg1                                                     [string] [required]

# default (help)
# yargs('--help')

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]
  --arg1     arg1 desc (default command)                     [string] [required]
  --arg2     arg2 desc (default command)                                [string]

*/

Solution

I'm still trying to figure out the fix for this. I wrote a unit test that should demonstrate the incongruity between expected/actual behavior.

Notes

This is what I've found so far, concerning failed default commands and the parts that are missing from the usage output:

  • validation functions call usage.fail if validation fails
  • usage.fail calls yargs.showHelp('error')
  • yargs.showHelp calls this.#usage.showHelp
  • usage.showHelp calls emit(self.help())

I logged descriptions at multiple points in the yargs process, and it's strange because arg1/arg2 show up in descriptions, but later get removed. I don't know if that's a result of resetting or using cached values, but something is wonky there. I'll keep trying to figure out why they get removed.

usage.describe { descriptions: { help: '__yargsString__:Show help' } }
usage.describe {
  descriptions: {
    help: '__yargsString__:Show help',
    version: '__yargsString__:Show version number'
  }
}
usage.describe { descriptions: { help: '__yargsString__:Show help' } }
usage.describe {
  descriptions: {
    help: '__yargsString__:Show help',
    version: '__yargsString__:Show version number'
  }
}
yargs.option { key: 'arg1', desc: 'arg1 desc (default command)' }
usage.describe {
  descriptions: {
    help: '__yargsString__:Show help',
    version: '__yargsString__:Show version number',
    arg1: 'arg1 desc (default command)'
  }
}
yargs.option { key: 'arg2', desc: 'arg2 desc (default command)' }
usage.describe {
  descriptions: {
    help: '__yargsString__:Show help',
    version: '__yargsString__:Show version number',
    arg1: 'arg1 desc (default command)',
    arg2: 'arg2 desc (default command)'
  }
}
usage.showHelp {
  descriptions: {
    help: '__yargsString__:Show help',
    version: '__yargsString__:Show version number'
  }
}
usage.help {
  descriptions: {
    help: '__yargsString__:Show help',
    version: '__yargsString__:Show version number'
  }
}

@jly36963
Copy link
Copy Markdown
Contributor Author

jly36963 commented Dec 31, 2021

@bcoe
The unfreeze call on this line is causing the weird behavior. It is wiping the descriptions and optional arguments for the default command. If I comment out that block, then I get the expected args/descriptions in the usage output. This would fix the issues reported in 2030 and 2102.

However, I don't think I understand the comment above that block.

yargs/lib/command.ts

Line 312 in 59a86fb

// if this is the case, we should show the root usage instructions

If I understand it correctly, then the usage shouldn't contain anything from the default command, eg: arg1. What should be the correct behavior here?

@jly36963 jly36963 marked this pull request as ready for review January 2, 2022 21:43
@jly36963 jly36963 requested a review from bcoe January 2, 2022 21:44
bcoe
bcoe reviewed Jan 9, 2022
View reviewed changes
Comment thread lib/usage.ts
descriptions,
} = frozen);
// Addresses: https://github.com/yargs/yargs/issues/2030
if (defaultCommand) {
Copy link
Copy Markdown
Member

@bcoe bcoe Jan 9, 2022

Choose a reason for hiding this comment

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

I like that you just made this a boolean setting rather than adding a new method.

@bcoe bcoe merged commit d6e342d into yargs:main Jan 9, 2022
@bcoe
Copy link
Copy Markdown
Member

bcoe commented Jan 9, 2022

@jly36963 as always, thank you for the contribution.

@github-actions github-actions Bot mentioned this pull request Jan 9, 2022
Merged
@jly36963 jly36963 mentioned this pull request Apr 1, 2022
Closed
This was referenced Sep 6, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bcoe bcoe bcoe approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jly36963 @bcoe