From 8e7405bf491eabb449e9c81627849d396229461d Mon Sep 17 00:00:00 2001 From: Bob Hyman Date: Tue, 23 May 2023 07:24:39 -0400 Subject: [PATCH] std dirs simply stays in sync with CD changes to PWD (#9267) # Description Fixes https://github.com/nushell/nushell/issues/9229. Supersedes #9234 The reported problem was that `shells` list of active shells (a.k.a `std dirs show` would show an inaccurate active working directory if user changed it via `cd` command. The fix here is for the `std dirs` module to let `$env.PWD` mask the active slot of `$env.DIRS_LIST`. The user is free to invoke CD (or write to `$env.PWD`) and `std dirs show` will display that as the active working directory. When user changes the active slot (via `n`, `p`, `add` or `drop`) `std dirs` remembers the then current PWD in the about-to-be-vacated active slot in `$env.DIRS_LIST`, so it is there if you come back to that slot. # User-Facing Changes None. It just works:tm: # Tests + Formatting # After Submitting --- crates/nu-std/lib/dirs.nu | 45 +++++++----- crates/nu-std/tests/test_dirs.nu | 117 ++++++++++++++++++++++++++----- 2 files changed, 127 insertions(+), 35 deletions(-) diff --git a/crates/nu-std/lib/dirs.nu b/crates/nu-std/lib/dirs.nu index 6aebca2e14..3154419935 100644 --- a/crates/nu-std/lib/dirs.nu +++ b/crates/nu-std/lib/dirs.nu @@ -1,6 +1,8 @@ -# Maintain a list of working directories and navigates them +# Maintain a list of working directories and navigate them # the directory stack +# current slot is DIRS_POSITION, but that entry doesn't hold $PWD (until leaving it for some other) +# till then, we let CD change PWD freely export-env { let-env DIRS_POSITION = 0 let-env DIRS_LIST = [($env.PWD | path expand)] @@ -22,9 +24,9 @@ export def-env add [ } let-env DIRS_LIST = ($env.DIRS_LIST | insert ($env.DIRS_POSITION + 1) $abspaths | flatten) - let-env DIRS_POSITION = $env.DIRS_POSITION + 1 - _fetch 0 + + _fetch 1 } export alias enter = add @@ -51,13 +53,12 @@ export alias p = prev # PWD becomes the next working directory export def-env drop [] { if ($env.DIRS_LIST | length) > 1 { - let-env DIRS_LIST = ( - ($env.DIRS_LIST | take $env.DIRS_POSITION) - | append ($env.DIRS_LIST | skip ($env.DIRS_POSITION + 1)) - ) + let-env DIRS_LIST = ($env.DIRS_LIST | reject $env.DIRS_POSITION) + if ($env.DIRS_POSITION >= ($env.DIRS_LIST | length)) {$env.DIRS_POSITION = 0} } - _fetch 0 + _fetch -1 --forget_current # step to previous slot + } export alias dexit = drop @@ -66,9 +67,12 @@ export alias dexit = drop export def-env show [] { mut out = [] for $p in ($env.DIRS_LIST | enumerate) { + let is_act_slot = $p.index == $env.DIRS_POSITION $out = ($out | append [ [active, path]; - [($p.index == $env.DIRS_POSITION), $p.item] + [($is_act_slot), + (if $is_act_slot {$env.PWD} else {$p.item}) # show current PWD in lieu of active slot + ] ]) } @@ -102,15 +106,24 @@ export alias g = goto # fetch item helper def-env _fetch [ - offset: int, # signed change to position + offset: int, # signed change to position + --forget_current # true to skip saving PWD ] { + if not ($forget_current) { + # first record current working dir in current slot of ring, to track what CD may have done. + $env.DIRS_LIST = ($env.DIRS_LIST | upsert $env.DIRS_POSITION $env.PWD) + } + + # figure out which entry to move to # nushell 'mod' operator is really 'remainder', can return negative values. # see: https://stackoverflow.com/questions/13683563/whats-the-difference-between-mod-and-remainder - let pos = ($env.DIRS_POSITION - + $offset - + ($env.DIRS_LIST | length) - ) mod ($env.DIRS_LIST | length) - let-env DIRS_POSITION = $pos + let len = ($env.DIRS_LIST | length) + mut pos = ($env.DIRS_POSITION + $offset) mod $len + if ($pos < 0) { $pos += $len} - cd ($env.DIRS_LIST | get $pos ) + # if using a different position in ring, CD there. + if ($pos != $env.DIRS_POSITION) { + $env.DIRS_POSITION = $pos + cd ($env.DIRS_LIST | get $pos ) + } } diff --git a/crates/nu-std/tests/test_dirs.nu b/crates/nu-std/tests/test_dirs.nu index 23866a578e..612982f0ff 100644 --- a/crates/nu-std/tests/test_dirs.nu +++ b/crates/nu-std/tests/test_dirs.nu @@ -1,8 +1,24 @@ +use std assert use std "assert length" use std "assert equal" +use std "assert not equal" +use std "assert error" +use std "log info" +use std "log debug" + +# A couple of nuances to understand when testing module that exports environment: +# Each 'use' for that module in the test script will execute the export def-env block. +# PWD at the time of the `use` will be what the export def-env block will see. export def setup [] { - {base_path: ($nu.temp-path | path join $"test_dirs_(random uuid)")} + # need some directories to play with + let base_path = ($nu.temp-path | path join $"test_dirs_(random uuid)") + let path_a = ($base_path | path join "a") + let path_b = ($base_path | path join "b") + + mkdir $base_path $path_a $path_b + + {base_path: $base_path, path_a:$path_a, path_b: $path_b} } export def teardown [] { @@ -12,44 +28,107 @@ export def teardown [] { rm -r $base_path } +def cur_dir_check [expect_dir, scenario] { + log debug $"check dir ($expect_dir), scenario ($scenario)" + assert equal $expect_dir $env.PWD $"expected not PWD after ($scenario)" +} +def cur_ring_check [expect_dir:string, expect_position: int scenario:string] { + log debug $"check ring ($expect_dir), position ($expect_position) scenario ($scenario)" + assert ($expect_position < ($env.DIRS_LIST | length)) $"ring big enough after ($scenario)" + assert equal $expect_position $env.DIRS_POSITION $"position in ring after ($scenario)" +} + export def test_dirs_command [] { - # need some directories to play with - let base_path = $in.base_path - let path_a = ($base_path | path join "a") - let path_b = ($base_path | path join "b") + # careful with order of these statements! + # must capture value of $in before executing `use`s + let $c = $in - mkdir $base_path $path_a $path_b + # must set PWD *before* doing `use` that will run the export def-env block in dirs module. + cd $c.base_path - cd $base_path + # must execute these uses for the UOT commands *after* the test and *not* just put them at top of test module. + # the export def-env gets messed up use std "dirs next" use std "dirs prev" use std "dirs add" use std "dirs drop" use std "dirs show" - - assert length $env.DIRS_LIST 1 "list is just pwd after initialization" - assert equal $base_path $env.DIRS_LIST.0 "list is just pwd after initialization" + use std "dirs goto" + + assert equal [$c.base_path] $env.DIRS_LIST "list is just pwd after initialization" dirs next - assert equal $base_path $env.DIRS_LIST.0 "next wraps at end of list" + assert equal $c.base_path $env.DIRS_LIST.0 "next wraps at end of list" dirs prev - assert equal $base_path $env.DIRS_LIST.0 "prev wraps at top of list" + assert equal $c.base_path $env.DIRS_LIST.0 "prev wraps at top of list" - dirs add $path_b $path_a - assert equal $path_b $env.PWD "add changes PWD to first added dir" + dirs add $c.path_b $c.path_a + assert equal $c.path_b $env.PWD "add changes PWD to first added dir" assert length $env.DIRS_LIST 3 "add in fact adds to list" - assert equal $path_a $env.DIRS_LIST.2 "add in fact adds to list" + assert equal $c.path_a $env.DIRS_LIST.2 "add in fact adds to list" dirs next 2 - assert equal $base_path $env.PWD "next wraps at end of list" + # assert (not) equal requires span.start of first arg < span.end of 2nd + assert equal $env.PWD $c.base_path "next wraps at end of list" dirs prev 1 - assert equal $path_a $env.PWD "prev wraps at start of list" + assert equal $c.path_a $env.PWD "prev wraps at start of list" + cur_dir_check $c.path_a "prev wraps to end from start of list" dirs drop assert length $env.DIRS_LIST 2 "drop removes from list" - assert equal $base_path $env.PWD "drop changes PWD to next in list (after dropped element)" + assert equal $env.PWD $c.path_b "drop changes PWD to previous in list (before dropped element)" - assert equal (dirs show) [[active path]; [true $base_path] [false $path_b]] "show table contains expected information" + assert equal (dirs show) [[active path]; [false $c.base_path] [true $c.path_b]] "show table contains expected information" +} + +export def test_dirs_next [] { + # must capture value of $in before executing `use`s + let $c = $in + # must set PWD *before* doing `use` that will run the export def-env block in dirs module. + cd $c.base_path + assert equal $env.PWD $c.base_path "test setup" + + use std "dirs next" + use std "dirs add" + cur_dir_check $c.base_path "use module test setup" + + dirs add $c.path_a $c.path_b + cur_ring_check $c.path_a 1 "add 2, but pwd is first one added" + + dirs next + cur_ring_check $c.path_b 2 "next to third" + + dirs next + cur_ring_check $c.base_path 0 "next from top wraps to bottom of ring" + +} + +export def test_dirs_cd [] { + # must capture value of $in before executing `use`s + let $c = $in + # must set PWD *before* doing `use` that will run the export def-env block in dirs module. + cd $c.base_path + + use std # necessary to define $env.config?? + + use std "dirs next" + use std "dirs add" + use std "dirs drop" + + cur_dir_check $c.base_path "use module test setup" + + cd $c.path_b + cur_ring_check $c.path_b 0 "cd with empty ring" + + dirs add $c.path_a + cur_dir_check $c.path_a "can add 2nd directory" + + cd $c.path_b + cur_ring_check $c.path_b 1 "cd at 2nd item on ring" + + dirs next + cur_ring_check $c.path_b 0 "cd updates current position in non-empty ring" + assert equal [$c.path_b $c.path_b] $env.DIRS_LIST "cd updated both positions in ring" }