Skip to content

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

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.

Sign up for GitHub

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

Open
qodelicious wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −2
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions modules/random-list/random-list.nu
View file
Open in desktop

@fdncred fdncred Oct 23, 2026

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.

Medium severity A count of 0 returns two items

The guards only reject < 0, so 0 gets through, and 1..0 counts down to [1, 0]. random-list bool 0 | length, random-list uuid 0 | length and random-list dice 0 | length all return 2 on 0.116.2. Every subcommand uses the same 1..$list_length loop (lines 29, 46, 60, 81, 95 and 108), and 0..<$list_length gives 0 items for 0 and n items for n. The message "must be greater than 0" also says 0 is invalid while the check accepts it; "must not be negative" matches the check. This was already there before the PR, but the description says every subcommand returns what it should.

nu_scripts/modules/random-list/random-list.nu

Lines 25 to 31 in 84629e8

if $list_length < 0 {
throw-error "value_error" "must be greater than 0" (metadata $list_length | get span)
}
1..$list_length | each {|it|
random bool --bias $bias
}

Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std/random

@fdncred fdncred Oct 23, 2026

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.

Medium severity Dice checks don't match std/random's dice

With this import, random dice is std's version, which asserts --dice > 0 and --sides > 1. The checks in random-list dice only reject negatives, so random-list dice 1 -r 0, -s 0 and -s 1 pass them and fail inside std, with the label at std/random/mod.nu:16/:17 instead of on the user's flag. Checking $roll_count < 1 and $side_count < 2, with messages that say so, keeps the error on the argument.

nu_scripts/modules/random-list/random-list.nu

Lines 74 to 79 in 84629e8

if $roll_count < 0 {
throw-error "value_error" "must be greater than 0" (metadata $roll_count | get span)
}
if $side_count < 0 {
throw-error "value_error" "must be greater than 0" (metadata $side_count | get span)
}


def throw-error [
error: string
msg: string
Expand All @@ -7,8 +9,7 @@ def throw-error [
msg: $"(ansi red_bold)($error)(ansi reset)"

@fdncred fdncred Oct 23, 2026

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.

Low severity ANSI codes end up in the error's msg

ansi red_bold and ansi reset are baked into the message string, so try { random-list bool -1 } catch {|e| $e.msg } returns "\e[1;31mvalue_error\e[0m", even with NO_COLOR=1. The error renderer already colors the headline, and msg: $error keeps $e.msg easy to match. This was already there before the PR.

label: {
text: $msg
start: $span.start
end: $span.end
span: $span

@fdncred fdncred Oct 23, 2026

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.

Low severity The same start/end label is still used in other scripts

modules/network/remoting/remoting.nu:76-77, sourced/filesystem/cdpath.nu:157-158 and sourced/fun/spark.nu:33 still pass start/end, which fails with "Value is missing required columns" on 0.116.2 when those errors fire. Fine as a follow-up if you'd rather keep this PR to random-list.

}
}
}
Expand Down
Loading