Keep SDKMAN_DIR from environment variables

reitzig / sdkman-for-fish

Adds support for SDKMAN! to fish

MIT License

280 stars 13 forks source link

Keep SDKMAN_DIR from environment variables #46

Closed xtexChooser closed 3 months ago

xtexChooser commented 6 months ago

sdkman-init.sh also do so

reitzig commented 6 months ago

Please explain shortly which problem this solves?

reitzig commented 6 months ago

Also, I've fixed the test setup; you'll note that you broke one of the tests with your change.

xtexChooser commented 5 months ago

when SDKMAN_DIR is defined through environment variable, it should not be over-written by sdk.fish.

reitzig commented 3 months ago

@xtexChooser Since you never responded, I'll close this for now.

If this is still relevant to you, please open an issue and explain:

Which setups are affected by ignoring pre-set SDKMAN_DIR? How do you set the environment variable?
Is the workaround with __sdkman_custom_dir (as mentioned in the docs) necessary at all?
If both variables are set, which value should take precedence, and why?

Until I have answers to these questions to mull over, I can't work off your PR to add/adapt tests.

xtexChooser commented 2 months ago

Hi, I set SDKMAN_DIR globally, through environment.d with system.

I think __sdkman_custom_dir may be redundant because SDKMAN_DIR is the way sdkman-cli is using to declare a custom installation location.

In my opinion, if both are defined, __sdkman_custom_dir should be preferred.

xtex@xtex ~ (main)> echo $SDKMAN_DIR
/home/xtex/.sdkman
xtex@xtex ~ (main)> sdk update
You don't seem to have SDKMAN! installed. Install now? [y/N] 
xtex@xtex ~ (main)> ls /home/xtex/.sdkman
ls: cannot access '/home/xtex/.sdkman': No such file or directory
xtex@xtex ~ (main) [2]> bash
xtex@xtex:~> echo $SDKMAN_DIR
/opt/sdkman
xtex@xtex:~> ls /opt/sdkman
bin  candidates  contrib  etc  ext  libexec  src  tmp  var

reitzig commented 2 months ago

Fix released with v2.1.0, thanks!

xtexChooser commented 2 months ago

Thanks!