Skip to content

fix: dont clobber description for multiple option calls#2171

Merged
bcoe merged 5 commits intoyargs:mainfrom
lukekarrys:lk/2169-clobber-descr
Jul 18, 2022
Merged

fix: dont clobber description for multiple option calls#2171
bcoe merged 5 commits intoyargs:mainfrom
lukekarrys:lk/2169-clobber-descr

Conversation

@lukekarrys
Copy link
Contributor

Fixes #2169

My original patch from #2169 broke some existing tests, due to it being necessary to call this.describe when initially setting up an option even if no explicit description is set.

So this PR implements this behavior by only calling this.describe if there is no existing description OR if the new description is a string.

I added a test that I confirmed fails on main and passes in this PR.

@jly36963
Copy link
Contributor

jly36963 commented May 6, 2022

Left one nit; otherwise, it looks great to me 👍

@jly36963 jly36963 self-assigned this May 6, 2022
@bcoe
Copy link
Member

bcoe commented May 16, 2022

@lukekarrys looks good to me, mind addressing @jly36963's one nit, and we'll work on merging?

@lukekarrys
Copy link
Contributor Author

@jly36963 @bcoe I pushed a fixup commit addressing the feedback, and I believe this should be good now

@jly36963
Copy link
Contributor

Looks good to me 👍

@bcoe bcoe merged commit f91d9b3 into yargs:main Jul 18, 2022
@bcoe
Copy link
Member

bcoe commented Jul 18, 2022

@lukekarrys thank you for the contribution, apologies for the terribly slow review.

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.

subsequent calls to options remove existing description

3 participants