cc: @Bahex and @132ikl

@fdncred While I'm confident that this is going to fix the nu -n issue, I'm still confused as to why your path is (was?) showing up as a string. This should fix that, but since I don't know the root cause of that one yet, I can't say with confidence. If it doesn't work for you after this PR, I'll dig into your config and see if I can reproduce.

My newly added tests are failing on Windows CI, so it looks like the conversion isn't working properly there. I'll test on Windows locally later. Moving to draft for now.

I hate it when everything passes except one platform. It's such a pain.

@fdncred It looks like these (new) failing tests might expose the problem you were having on Mac. Apparently the path conversion still wasn't case-insensitive as we thought, but we didn't have any test in place to cover that particular area.

With the new tests in place, it fails, and that's a good thing, IMHO.

Working on fixing it to make it truly case-insensitive now.

I just tried this PR and still doesn't work right. Hopefully you're on to something good.

"Case-insensitive" isn't the right word - I should get better about distinguishing "case-preserving". The problem here is that path conversions didn't preserve the case. So there were two environment variables (PATH and, e.g., Path). Most likely conversions were broken on every system that didn't use PATH.

Again, the tests are now in place to catch this (which is what kept this one from going green until I got the real fix in).

Can you give it another try?

I still get the same thing on my mac when using my regular configuration. If I use nu -n I get env vars in a list now.

This is what I have in my env.nu for ENV_CONVERSIONS.

export-env {
  let esep_list_converter = {
    from_string: { |s| $s | split row (char esep) }
    to_string: { |v| $v | str join (char esep) }
  }
  let esep_path_converter = {
    from_string: { |s| $s | split row (char esep) }
    to_string: { |v| $v | path expand | str join (char esep) }
  }

  $env.ENV_CONVERSIONS = {
    PATH: $esep_path_converter
    # Path: $esep_path_converter
    LS_COLORS: $esep_list_converter
    DYLD_FALLBACK_LIBRARY_PATH: $esep_path_converter
    # PATHEXT: $esep_list_converter
    # PSMODULEPATH: $esep_path_converter
  }
}

I have the same ENV_CONVERSIONS section on my Windows box, and $env.path (any text case) produces a list of paths on the latest main branch. This makes me wonder 2 things.

1. Are line endings involved?
2. Are path separators involved since mac uses : and windows uses ;

Also, one more tidbit.
On Windows, with my normal configuration, on the latest main branch, i get the path list. When I run the latest main branch with nu -n I do not get a list, which is kind of the opposite of mac right now. Let me try this PR on Windows and see what it does now.

On Windows, with this PR, nu -n and nu with my regular config files produce a $env path that is a list. I'm not sure what's going on with Mac and perhaps linux here.

Hmm. I have a hunch, but I'll have to wait until this afternoon to see if it's on the right track.

In the meantime, could you test without a path conversion in ENV_CONVERSIONS?

• Are line endings involved?
• Are path separators involved since mac uses : and windows uses ;

std::env::split_paths(val) is used, which I would think would handle all cases properly.

@Bahex I think I see the (or at least part of the) problem. #14591 seems to have the same problem I just fixed in ensure_path(). That is, it's not preserving case.

Steps to reproduce:

$env.foo = "foo"
$env.ENV_CONVERSIONS = {
  foo: {
    from_string: {|| '✅'}
    to_string: {|| 'N/A'}
  }
}
# Correct, since the case of the ENV_CONVERSION variable is the same:
print $"$env.foo should be ✅.  $env.foo is ($env.foo)"
# => $env.foo should be ✅.  $env.foo is ✅

# Also correct, since the lookup of $env.FOO should match case insensitive to $env.foo
print $"$env.FOO should be ✅.  $env.FOO is ($env.FOO)"
# => $env.FOO should be ✅.  $env.FOO is ✅

$env.bar = "❌"
$env.ENV_CONVERSIONS = {
  BAR: {
    from_string: {|| '✅'}
    to_string: {|| 'N/A'}
  }
}
# `bar` is not converted
print $"$env.bar should be ✅.  $env.bar is ($env.bar)"
# => $env.bar should be ✅.  $env.bar is ❌
# but `BAR` is a newly created variable by ENV_CONVERSIONS
print $"$env.BAR should be ✅.  $env.BAR is ($env.BAR)"
# => $env.BAR should be ✅.  $env.BAR is ✅

This will be an issue if anyone has a PATH conversion in ENV_CONVERSIONS that doesn't match the case being used by the system.

Not 100% sure that's what @fdncred is running into, but it seems likely given the repro above.

Ahh, just looked at the source code, I can see where I messed up, I do a case insensitive lookup, but store the new value with the key in ENV_COVERSIONS.
Sorry for the trouble 😅. At least it uncovered the nu -n case.

@Bahex No worries! Obviously it took me a while to find the (same) issue in ensure_path as well. Longstanding bug, apparently, that we're just now surfacing.

@fdncred Nice-to-have might be for get_env_var_insensitive() to return both the name and the value? That would allow us to fix this more centrally, I believe.

@fdncred Ok, I think it's fixed for real this time. Can you throw your config at it to test?

There's a chance that there's a breaking change here if you are setting an ENV_CONVERSION on PATH (or any case-variation) or treating it as a string anywhere in your config. I think it's still going to work regardless, but there's a chance ...

Still doesn't work but I think I figured out why based on your last comment, "or treating it as a string anywhere in your config".

I have the ENV_CONVERSIONS set as above but what I didn't think about until you mentioned it, is I do this in my config.nu to get rid of any duplicate entries in my path.

$env.PATH = ($env.PATH | par-each {|r| $r | split row (char esep)} | flatten | uniq | str join (char esep))

If I remove that, it returns path as a list. However, shouldn't we be able to assign items to $env.PATH and have them still return a list without having to do a bunch of list creation/append/prepending? Another way of saying it is that I've used this for a long time and it just recently stopped working.

I wonder if we should allow $env.path to be changed from a list to a different type. 🤔

However, shouldn't we be able to assign items to $env.PATH and have them still return a list without having to do a bunch of list creation/append/prepending?

In this case, you aren't modifying the list, though. You specifically convert it back from a list to a string there at the end, right?

str join (char esep)

Once an environment variable is converted, there isn't anything in place to convert it again. Going back to 0.97.1 (before I started mucking about with it ;-):

convert_env_values() was only called in three places:

• Once before running nu <script>
• Once before running nu -c <commandstring>
• Once before starting the REPL

Another way of saying it is that I've used this for a long time and it just recently stopped working.

Is it possible you just haven't been actively inspecting the type of $env.PATH? I just tested your statement above on 0.97.1, and it works the same way there (sets PATH back to a string):

let temp_home = (mktemp -d)
$env.XDG_CONFIG_HOME = $temp_home
$env.XDG_DATA_HOME = $temp_home
~/oldnu/nu-0.97.1-x86_64-unknown-linux-gnu/nu
# => No environment config file found at /tmp/tmp.r9CMRnZntc/nushell/env.nu
# => Would you like to create one with defaults (Y/n):
# => 
# => Environment config file created at: /tmp/tmp.r9CMRnZntc/nushell/env.nu
# => No config file found at /tmp/tmp.r9CMRnZntc/nushell/config.nu
# => Would you like to create one with defaults (Y/n):
# => 
# => Config file created at: /tmp/tmp.r9CMRnZntc/nushell/config.nu
# Banner     __  ,

$env.PATH | first 5
# => ╭───┬────────────────────────────────────────╮
# => │ 0 │ /home/ntd/.local/share/rust/cargo/bin  │
# => │ 1 │ /home/ntd/.local/share/rust/rustup/bin │
# => │ 2 │ /home/ntd/.config/carapace/bin         │
# => │ 3 │ /home/ntd/.nix-profile/bin             │
# => │ 4 │ /home/ntd/.nix-profile/bin             │
# => ╰───┴────────────────────────────────────────╯
$env.PATH = ($env.PATH | par-each {|r| $r | split row (char esep)} | flatten | uniq | str join (char esep))
$env.PATH | str substring 0..100
# => /home/ntd/.local/share/rust/cargo/bin:/home/ntd/.local/share/rust/rustup/bin:/home/ntd/.config/carapa
$env.PATH | describe
# => string

I wonder if we should allow $env.path to be changed from a list to a different type. 🤔

Probably not. Or perhaps there should be logic to run from_string whenever an environment variable is set back to a string?

It just doesn't seem that there's ever been any logic along those lines.

Is it possible you just haven't been actively inspecting the type of $env.PATH?

You'd think that things wouldn't work right if it was just a string.

Either way, I'm fine with landing this and seeing if anything else breaks. Good work researching and testing to figure out what was going on.

You'd think that things wouldn't work right if it was just a string.

I need to look up the code-path for that (to satisfy my own curiosity), but it seems to work fine as a string or a list:

$env.PATH = "/usr/bin:/usr/sbin"
^ls
# => Works
nu
# => Fails, since cargo/bin is no longer in the path

@fdncred Ah, we're using the which crate, which requires a delimited string anyway, so we convert back to a string (from a list, or a noop if already a string) before doing a lookup.

Possibly with another case-bug: Looks like this PATH vs. Path check is unnecessary, but works okay due to env_to_string().

Should be ready to land if it looks okay to everyone else.

Thanks