Skip to content

random-list: fix dice and the error helper on current nushell - #1301

Merged
fdncred merged 2 commits into
nushell:mainfrom
qodelicious:fix-random-list-dice
Oct 24, 2026
Merged

fdncred merged 2 commits into
nushell:mainfrom
qodelicious:fix-random-list-dice

Conversation

@qodelicious

Copy link
Copy Markdown
Contributor

random-list dice didn't parse on current nushell because random dice moved out of the builtins into std/random. The module now does use std/random at the top.

While testing that I also hit the error helper failing with "missing span column", since error make wants span in the label now instead of separate start and end. It passes the span through directly so the validation errors show up again.

Tested on 0.116.1. nu --ide-check is clean, every random-list subcommand returns what it should, and passing a negative count or a bias above 1 shows the right error pointing at the argument.

@fdncred fdncred left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review overview

🟡 Changes recommended

The std/random import and the span change work on 0.116.2. What's left is argument checks that let 0 through.

Findings: 0 High severity · 2 Medium severity · 2 Low severity

Open (4)

Notes from my clanker, checked on 84629e8 with nu 0.116.2. I ran every random-list subcommand with valid input, 0, negative counts, an out-of-range --bias, and dice counts of 0 and 1.

Comment thread modules/random-list/random-list.nu Outdated
Comment thread modules/random-list/random-list.nu
Comment thread modules/random-list/random-list.nu
Comment thread modules/random-list/random-list.nu
Dice checks now match std's random dice (at least 1 die, at least 2
sides) so the error points at the flag. A count of 0 gives an empty
list instead of 2 items, and the error msg no longer has ansi codes in it.
@fdncred
fdncred merged commit 02cb1a2 into nushell:main Oct 24, 2026
1 check passed
@fdncred

fdncred commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Thanks

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.

2 participants