dotnet-env icon indicating copy to clipboard operation
dotnet-env copied to clipboard

Add option to disable Interpolation

Open Philipp-Binder opened this issue 1 year ago • 6 comments

I've added an option to disable the Interpolation. Main reasons:

  • the current Interpolation-Implementation stops working if you do not set EnvironmentVariables (see #92) ==> it is good to have the option to disable it completely
  • Interpolation adds complexity which is not needed in every setup ==> good to have the option to reduce complexity

It was way harder to achieve than expected, because the Interpolation is so deep inside the static Parsers. Changing Assignment to be a method instead of a readonly field seems to be the least intrusive way. There is also this test in EnvTests-->ParseInterpolationDisabledTest line 224. I think this is not the really desired result, but I do not see a way how to solve that.

Philipp-Binder avatar Feb 07 '24 20:02 Philipp-Binder

Please explain why you think interpolation should be disabled. You are afraid that you might use interpolation in your .env when you don't mean to?

SetEnvVar = false does not provide interpolated values that come from the .env file but it should continue to work with env vars that were set in the env itself.

But this further points at: if you don't want interpolation, don't use it. You can escape $ in unquoted or double quoted values, and will not get interpolation in single quoted values ever.

Please elaborate on the use case.

rogusdev avatar Mar 02 '24 05:03 rogusdev

Yes, the use case is that you just want to have plain env-file to be able to configure a service for local usage and don't bother with any side-effects.

But this further points at: if you don't want interpolation, don't use it. You can escape $ in unquoted or double quoted values, and will not get interpolation in single quoted values ever.

Sure, it is possible to escape $ or use singlequoted. But then you have to have it in mind and can't just paste your values exactly as they are meant to be.

The main use cases I see are "complicated" values like passwords, where you do not want to check or change anything to enter the password in the env-file. Passwords may contain ' and $, so depending on the form of the password you have the following options:

  • contains ' --> unquoted (escaping singeQuotes in singleQuoted values is not possible)
  • contains $ --> use singleQuotes or use unquoted and escape $
  • contains both --> use unquoted and escape $

Escaping values in a complicated value is quite a pain, because you create differences between the written and the resulting value. And imho it introduces hard to detect bugs.

==> the idea is to have the possibility to use env-Files "as plain and stupid as possible" From my point of view that's the thing I want to achieve. All additional features are cool if you really need them, but in general I don't want to bother with additional complexity for a smart and easy local configuration only.

And "as plain and stupid as possible" includes (my opinion):

  • unquoted values
  • quoted values (single/double) to support multiline values
  • comments
  • simple dictionary-output And that's it. Interpolation, updating (Process-)EnvironmentVariables etc. are nice to have, but not really necessary. I would not remove them from the solution of course, but the usage should be optional.

Philipp-Binder avatar Mar 02 '24 11:03 Philipp-Binder

Anything that should not have interpolation should be single quoted. You haven't said (yet) that you want to disable utf char sequences as well \u1234 etc, or escaped chars \n etc. Why stop with just disabling $s? You should disable all special processing -- oh wait, you can, that's single quoted values. ;)

That said, your PR does seem to me to make the change you desire, with the only minor consideration what I mentioned about a possible RawValue to avoid creating both interpolated and raw versions every time, and instead only the one that is known to be desired.

rogusdev avatar Mar 02 '24 11:03 rogusdev

You also have a failing test, per the CI

rogusdev avatar Mar 02 '24 11:03 rogusdev

That Test is a quite ugly thing... and it is related to the implementation of EnvVar-Usage.

If you execute the Test locally (single test) it works as expected. But if you concurrently execute other tests it will break. But if you execute them all at once, it fails locally too.

It seems like a "race-condition" with EnvConfigurationTests.Dispose(), which is quite reliable triggered with my additional Test-method if you execute all tests at once.

This has to be fixed of course. I think I have to find a way how to separate the executions properly, or just duplicate the .env_embedded file and change some variable-names to remove concurrent keys.

edit: it it not related to EnvConfigurationTests.Dispose() I guess after having a second look. The Env-Values set by my additional TestMethod are being used within the previously existing method. It's all about the direct dependencies to EnvironmentVariables. It would be a great thing to be able to mock that for testing purposes, but I do not see a way to achieve that with the current solution.

--> I will search for a way to make the tests work concurrently soon, but that should not affect the other aspects of this MR

Philipp-Binder avatar Mar 02 '24 19:03 Philipp-Binder

You also have a failing test, per the CI

Solved now.

Philipp-Binder avatar Mar 17 '24 13:03 Philipp-Binder

Having thought about this more: The way to disable interpolation is, as it is in normal shell scripting and so on, with single quotes. That is the path to use, and adding new options to do the same thing does not have a compelling argument here for me, not to justify this increase in complexity.

If you can find other .env libs in other langs that have such an option, I would consider it for ease of moving between languages for users. For now, no.

rogusdev avatar Jul 30 '24 14:07 rogusdev

Also, if this was desirable, the solution would be to use the already purpose built ValueActual rather than adding _raw to ValueInterpolated. This would require having a swap for the Value Parser used https://github.com/tonerdo/dotnet-env/blob/18051317d2ddd87d2dc075c8a55e98cf35fce6e1/src/DotNetEnv/Parsers.cs#L235 such that there is the current Value still and then NonInterpolatedValue -- which would in turn go down to the lower parsers and remove the interpolation option from the UnquotedValueContents and DoubleQuotedValueContents -- and in the ParseDotenvFile func, you would replace Assignment with NonInterpolatedAssignment that would use the new form of Value. But then you'd have to ask hard questions about the other interpreted char sequences in those values as well: special chars / escaped utf codepoints, newlines, tabs, etc, etc. And then you've lost the point of double and unquoted values.

The way unquoted and double quoted work at present is how they work in shells, which is what people are accustomed to in other .env libs as well, to my knowledge (I've only used such libs in 2 or 3 other languages, so idk for sure, obv). Making these kinds of changes would be unnecessarily complicating things, to my perspective.

rogusdev avatar Jul 30 '24 14:07 rogusdev