some tweaks for a variety of scripts #1191
Conversation
There was a problem hiding this comment.
Pull request overview
This PR contains several script improvements that have been accumulated over time, including:
- Updating to the current nushell standard for home directory reference
- Refactoring color sample generation with better code organization
- Expanding color code ranges to include additional ANSI codes
- Replacing polars dependency with native nushell commands
Key Changes
- Replaced deprecated
$nu.home-pathwith$nu.home-diracross prompt module - Refactored
modules/coloring/sample.nuto use a helper function and wrap main logic in a function - Extended ANSI color code ranges (30..39, 40..49, 90..97, 100..107) with filtering to exclude codes 38 and 48
- Migrated
make_release/20k_club.nufrom polars to native nushell table operations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/prompt/oh-my.nu | Updated 4 occurrences of deprecated $nu.home-path to $nu.home-dir |
| modules/coloring/sample.nu | Added into list helper function, wrapped code in print_sample function, adjusted ANSI color code iteration order |
| modules/coloring/color_tables.nu | Extended color ranges and added filtering for reserved ANSI codes 38 and 48 |
| modules/coloring/color_table.nu | Extended color ranges, added filtering for codes 38 and 48, cleaned up formatting, added code 49 to header |
| make_release/20k_club.nu | Replaced polars operations with native nushell commands (group-by, upsert, sort-by) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let row = (30..39 | each {|i| | ||
| let row = (40..49 | each {|a| | ||
| if ($a != 48) { | ||
| if ($i != 38) { |
There was a problem hiding this comment.
Inconsistent indentation: the inner if statement on line 17 should be indented to match the nesting level inside the outer if block. The code block on lines 18-19 is correctly indented relative to the second if, but line 17 itself appears to be at the wrong indentation level.
| if ($i != 38) { | |
| if ($i != 38) { |
| } | str join | ||
| } | str join | ||
| } | ||
| print_sample |
There was a problem hiding this comment.
[nitpick] Trailing whitespace after the function call. Remove the space after print_sample.
| print_sample | |
| print_sample |
| (make_header false) | ||
| # (mk_header 40..49) | ||
| (create_color_tables 30..39 40..49) | ||
|
|
||
| # put a line between tables | ||
| (char newline) | ||
|
|
||
| #make_header $true | ||
| (mk_header 100..107) | ||
| (make_header true) | ||
| # (mk_header 100..107) |
There was a problem hiding this comment.
[nitpick] Inconsistent use of header functions: both make_header and mk_header are defined but only make_header is being used (lines 81, 88), while mk_header calls are commented out (lines 82, 89). Consider removing the unused mk_header function or documenting why both are kept, as noted in the comment on lines 27-29.
|
|
||
| def print_sample [] { | ||
| # Background Colors | ||
| [40..47 49 100..107] | each { into list } | flatten | each { |clbg| |
There was a problem hiding this comment.
The order of background colors has changed from the bash reference implementation. The bash script (line 48) iterates as {40..47} {100..107} 49 (ranges 40-47, 100-107, then 49), but the new code on line 15 uses [40..47 49 100..107] (ranges 40-47, value 49, then 100-107). This will produce output in a different order. If the order matters for the expected output, consider reordering to match: [40..47 100..107 49].
| [40..47 49 100..107] | each { into list } | flatten | each { |clbg| | |
| [40..47 100..107 49] | each { into list } | flatten | each { |clbg| |
| # Background Colors | ||
| [40..47 49 100..107] | each { into list } | flatten | each { |clbg| | ||
| # Foreground Colors | ||
| [30..37 39 90..97] | each { into list } | flatten | each { |clfg| |
There was a problem hiding this comment.
The order of foreground colors has changed from the bash reference implementation. The bash script (line 50) iterates as {30..37} {90..97} 39 (ranges 30-37, 90-97, then 39), but the new code uses [30..37 39 90..97] (ranges 30-37, value 39, then 90-97). This will produce output in a different order. If the order matters for the expected output, consider reordering to match: [30..37 90..97 39].
| [30..37 39 90..97] | each { into list } | flatten | each { |clfg| | |
| [30..37 90..97 39] | each { into list } | flatten | each { |clfg| |
| let input = $in | ||
| let type = ($input | describe --detailed | get type) | ||
| match $type { | ||
| range => {$input | each {||}} |
There was a problem hiding this comment.
[nitpick] The empty closure {||} on line 5 is an unusual way to convert a range to a list. Consider using a more idiomatic approach such as each {|x| $x} or wrapping the range in a list: [$input] (for single ranges) or using the range directly since most nushell commands automatically enumerate ranges. The current syntax may work but is not conventional.
| range => {$input | each {||}} | |
| range => {$input | each {|x| $x}} |
1 participant
These are some script tweaks that I've been holding onto for too long and am finally ready to get them into nu_scripts.