std dirs simply stays in sync with CD changes to PWD (#9267)

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
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
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
None. It just works™️

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> [x] use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

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 = (
+        let-env DIRS_LIST = ($env.DIRS_LIST | reject $env.DIRS_POSITION)
-            ($env.DIRS_LIST | take $env.DIRS_POSITION) 
+        if ($env.DIRS_POSITION >= ($env.DIRS_LIST | length)) {$env.DIRS_POSITION = 0}
-            | append ($env.DIRS_LIST | skip ($env.DIRS_POSITION + 1))
-        )
     }
 
-    _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 
+    let len = ($env.DIRS_LIST | length)
-                + $offset 
+    mut pos = ($env.DIRS_POSITION + $offset) mod $len
-                + ($env.DIRS_LIST | length)
+    if ($pos < 0) { $pos += $len}
-            ) mod ($env.DIRS_LIST | length)
-    let-env DIRS_POSITION = $pos
 
-    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 )
+    }
 }

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
+    # careful with order of these statements!
-    let base_path = $in.base_path
+    # must capture value of $in before executing `use`s
-    let path_a = ($base_path | path join "a")
+    let $c = $in    
-    let path_b = ($base_path | path join "b")
 
-    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"
-
+    use std "dirs goto"
-    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"
+    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
+    dirs add $c.path_b $c.path_a
-    assert equal $path_b $env.PWD "add changes PWD to first added dir"
+    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"
 }