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

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

Merged
merged 4 commits into from Jan 9, 2022

Conversation

jly36963
Copy link
Contributor

@jly36963 jly36963 commented Dec 27, 2021

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
Contributor Author

@jly36963 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.

// 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 Jan 2, 2022
@jly36963 jly36963 requested a review from bcoe Jan 2, 2022
descriptions,
} = frozen);
// Addresses: https://github.com/yargs/yargs/issues/2030
if (defaultCommand) {
Copy link
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 approved these changes Jan 9, 2022
@bcoe bcoe merged commit d6e342d into yargs:main Jan 9, 2022
7 checks passed
@bcoe
Copy link
Member

@bcoe bcoe commented Jan 9, 2022

@jly36963 as always, thank you for the contribution.

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.

None yet

2 participants