Environment.ExpandEnvironmentVariables on Linux has Windows behavior

[FeodorFitsner]

Environment.ExpandEnvironmentVariables() on Linux expands variables in %VAR_NAME% format rather than $VAR_NAME.

SDK version: 2.1.4
OS: Ubuntu 16.04

A simple test:

Environment.ExpandEnvironmentVariables("$HOME/test");

Expected value: /home/{user}/test
Actual value: $HOME/test

However running:

Environment.ExpandEnvironmentVariables("%HOME%/test");

gives expanded result /home/{user}/test.

[danmoseley]

My guess is this was done intentionally -- so existing code passing %VAR_NAME% "just works" when used on Unix. For code written for Unix, it is certainly unexpected.
The API could also respect $ - if it is worth diverging further and becoming potentially confusing.
Another option is an API

public string Environment.ExpandEnvironmentVariables(string name, EnvironmentVariablesExpandFlavor flavor = EnvironmentVariablesExpandFlavor.PlatformDefault);

[Flags]
public enum EnvironmentVariablesExpandFlavor
{
           PlatformDefault = 0x1,
           PercentSigns = 0x2,
           DollarSigns = 0x4
}

with better names...

Maybe @stephentoub recalls context.

[stephentoub]

Maybe @stephentoub recalls context.

Mainly because that's how it's documented:
https://docs.microsoft.com/en-us/dotnet/api/system.environment.expandenvironmentvariables

A string containing the names of zero or more environment variables. Each environment variable is quoted with the percent sign character (%).

I could be convinced that you'd never do that on unix and this is really just surfacing a Windows-ism and so we should use $ on unix instead of %. But we'd want to understand how badly breaking that would be. I'd be worried that code has already done string.Replace('$','%') to make it work the desired way on unix, and then we'd be breaking those folks, for example.

[joshfree]

I think we should add a new overload that supports this.

I'd be worried that code has already done string.Replace('$','%') to make it work the desired way on unix,

ps - string.Replace('$', '%') would add $ suffixes to your expanded env vars 😉

[gfoidl]

For cross-platform development both variants should be allowed, similar to the path-separator (\ vs. /).

Batch allows the $ as valid variable name, but then it would have to be quoted by %, so %$%. This case can be handled.
In Bash the % isn't allowd in variable names, so this case can be handled too.

Further there wouldn't be any breaking changes, since %var% and $var would work.

[stephentoub]

Further there wouldn't be any breaking changes, since %var% and $var would work.

Except $var on Windows may start to be reinterpreted, changing output in some cases.

[aaronfranke]

It may also be worth considering expanding ~. I know that it is used by some programs in file names, so I'd suggest only expanding it if it's the first character in the given path string.

[RickStrahl]

Maybe a whole new API should be used for this because ExpandEnvironmentVariables doesn't sound like it's necessarily path specific.

Personally I would prefer something like FixupOsPath(path) or ExpandOsPath() that fixes up any path that could be used by the operating system directly. This would include the appropriate platform specific environment vars, as well as root folder expansion (ie. ~) and anything else that might be available for a specific Os target.

[LethiferousMoose]

Maybe I am misunderstanding the complexity of Environment.ExpandEnvironmentVariables(), but couldn't the code just do an OS check (Windows, Mac, Unix, etc.) and look for the proper symbols per OS?

[tanith87]

Maybe a whole new API should be used for this because ExpandEnvironmentVariables doesn't sound like it's necessarily path specific.

Because it is not path specific. Environment variables can be anything. Not only paths.

Maybe I am misunderstanding the complexity of Environment.ExpandEnvironmentVariables(), but couldn't the code just do an OS check (Windows, Mac, Unix, etc.) and look for the proper symbols per OS?

This would certainly be a breaking change for any code that already relies on the %VAR% pattern.