Supports IntelliJ PathMacro like $PROJECT_DIR$

[yoanthiebault]

Supports IntelliJ PathMacro like $PROJECT_DIR$ in environment variable values. It allows the user to write TEST=$PROJECT_DIR$/test.txt in his env file instead of TEST=/path/to/project/test.txt

PathMacroManager was found at https://intellij-support.jetbrains.com/hc/en-us/community/posts/206781805-Working-with-PROJECT-DIR-in-plugin-configuration-options

[ashald]

Why is this needed given that EnvFile supports relative paths?

[yoanthiebault]

I need to set an environment variable containing absolute paths to some of my project files (for instance TEST=$PROJECT_DIR$/test.txt). Do you mean it's already possible to do that with the plugin ?

[ashald]

Ugh, my bad - I misunderstood the intent.

Given that the plugin now supports environment variable interpolation I believe it'd be better to approach this feature in a different way. I'd rather keep the "core" functionality as limited as possible (like, set env vars from an ordered list of sources with an optional interpolation) and rather provide extra features with another level of extensions.

For instance, right now plugin supports "yaml/json" and ".env" files as sources as well as "env vars defined in run configuration". All of those "sources" are implemented as extensions on their own. You can find interfaces and implementations in "core" module. So I'd rather suggest implementing this feature just as another "source". Does that make sense?

[yoanthiebault]

No problem, maybe my description wasn't enough clear :)

Do you mean I should create a new type of "source" with a new extension (different from .env/.yaml/.json) and don't change the behavior of all already existing sources (by updating AbstractEnvVarsProvider for example) ?

If yes I'm not 100% sure it's a good idea. PathMacro variables like $PROJECT_DIR$ are builtin Intellij features. For instance, if you add manually the environment variable TEST=$PROJECT_DIR$/example.txt to a runtime configuration in the user interface, IntelliJ will automatically replace $PROJECT_DIR$ by the project directory absolute path. This is why I think PathMacro variables should be supported by all sources and not by a specific type of source. What do you think ?

[yoanthiebault]

And by the way, the only two interesting lines in the Pull Request are:

PathMacroManager macroManager = PathMacroManager.getInstance(project);
String inlinedEnvVar = macroManager.expandPath(envVar);

They could be moved to EnvFileConfigurationEditor.collectEnv if you want to keep core as simple as possible.

[ashald]

Yes, I was trying to keep core free from dependencies on JetBrains platform. Not sure if it's wort it in long run though.
I wonder if such a feature should be optional so that those who don't want macros to be expanded can opt out (e.g., they need to set value of an env var to $PROJECT_DIR$) as it is with substitution.

And while we're discussing this, given that you mentioned that such macros are processed in run configuration env vars I believe you can set an env var PROJECT_DIR=$PROJECT_DIR$ on run configuration tab and then rely on env var substitution and just refer to ${PROJECT_DIR} in your env var files to get the absolute path as a workaround.

[yoanthiebault]

So, I updated the pull request:

• I removed the changes from core in order to make it free from JetBrains platform dependencies
• I made the path macro support optional but it's enabled by default. Indeed, I think it's important that default settings stick as mush as possible to JetBrains IDE behavior

I believe you can set an env var PROJECT_DIR=$PROJECT_DIR$ on run configuration tab and then rely on env var substitution and just refer to ${PROJECT_DIR} in your env var files to get the absolute path as a workaround.

-->This workaround would work but is not really user-friendly. I think we can do better :)

What do you think about the new modifications ?

[ashald]

The PR looks good to me!

I have 2 concerns though that I'd like to discuss with you.

1. For people using .env files not only with JetBrains IDEs but with other tools like foreman/honcho the concept of "path macro" can be foreign and therefore I'm not sure it's a good idea to enable it by default. I'd rather kept it turned off by default. It's always possible to adjust default value in run configuration templates.
2. We perform env var substitution on every step in a loop but macro substitution only once at the very end. Should we move it into the loop so that we would perform it each time after a file entry was processed?

[yoanthiebault]

So:

1. Just updated the PR
2. Moving the path macro substitution inside the file entry loop would be less optimized than the current solution (as we would re-substitute every env variables on each file entry) and doesn't change the functional behavior. So I don't think it's really useful to do it. But if you really want I still can update this part of the code. In any cases there's not much difference.

[ashald]

Thanks!

With regards your 2nd point, I can imagine a situation like this:

1st file defines ROOT=%PROJECT_DIR%
2nd file defines CONFIG_DIR=${ROOT}/etc
3rd file defines SOME_CONFIG_FILE=${CONFIG_DIR}/my.conf

This would not work with current implementation but will be processed as expected if we will move macros expansion into the loop. I don't think we need to worry about optimizations here as it's relatively cheap operation and it's not like we're chasing milliseconds here.

[yoanthiebault]

I don't get the point. You provided this example:

1st file defines ROOT=%PROJECT_DIR%
2nd file defines CONFIG_DIR=${ROOT}/etc
3rd file defines SOME_CONFIG_FILE=${CONFIG_DIR}/my.conf

But with the current implementation we would have:

After processing 1st file:

ROOT=%PROJECT_DIR%

After processing 2nd file:

ROOT=%PROJECT_DIR%
CONFIG_DIR=%PROJECT_DIR%/etc

After processing 3rd file:

ROOT=%PROJECT_DIR%
CONFIG_DIR=%PROJECT_DIR%/etc
SOME_CONFIG_FILE=%PROJECT_DIR%/etc/my.conf

Then the path macro substitution would replace all the %PROJECT_DIR% and return:

ROOT=/path/to/project/dir
CONFIG_DIR=/path/to/project/dir/etc
SOME_CONFIG_FILE=/path/to/project/dir/etc/my.conf

[ashald]

Ugh, you're right, I missed that. Thanks!

OK, last thing before merging the PR - would you mind adding yourself to AUTHORS.md?

[yoanthiebault]

Done
ps: I just added my last name, my name was already in AUTHORS.md ;)

[ashald]

Ahaha, a returning contributor. 🙂 That I did not envision. Thanks!

[yoanthiebault]

You're welcome ! :)

[yoanthiebault]

By the way, when do you think release and publish the last modifications ?

[ashald]

Meh, we forgot to update changelog. :(
Will cut a release today, will take few days for JetBrains to publish it.

[yoanthiebault]

Ok! Thank you!