FelixSelter / JEnv-for-Windows

Change your current Java version with one line
Apache License 2.0
676 stars 87 forks source link

Use `.java-version` file to determine java version #37

Closed lkleeven closed 1 year ago

lkleeven commented 2 years ago

Potential solution for #36

lkleeven commented 2 years ago

I currently went for the smaller approach of trying of just adding reading the file. It could be extended by completely replacing the current use of the config file in favor of the .java-version file.

FelixSelter commented 2 years ago

Hi, This looks like an excellent feature for jenv. Thank you for your contribution.

However, I think we should change the order of selecting the version.

If the .java-version file is preferred before the local version the user cannot override the preferences. The user may want to override them if he named his installation different than the repo. Or he may want to test a newer version.

This could obviously be achieved by changing the contents of the .jenv-version file. This approach seems tedious compared to executing a command via the command line and could lead to accidentally breaking git pushes. The user may be warned by a message that the version specified inside the .java-version file does not match the configured jenv local.

As far as I can tell from the JEnv Readme the .java-version file contains only the version string 11.0.2 jenv-getjava.psm1 however is expected to return a path to a java home C:/Program Files/Java/jre11.0.2 If I get this correct your code does not take care of this and just returns the contents of the file.

How I think this feature should be implemented:

Do you think you could implement this or have a better idea?

lkleeven commented 2 years ago

Why would you want to put the automatic version resolving all the way to the end? I would expect most users to have a global version picked, so any time the file is present and the global doesn't match the version in the file we would just give a warning. That doesn't really sound very user friendly.

By the way the other JEnv only puts the major version in the .java-version file. If I can make the following suggestion for this feature and maybe make it a bit bigger:

  1. When adding a java runtime determine the version and store it in the config file
  2. For backwards compatibility add a utility to add the versions in case they aren't there yet
  3. When determining the java version to use:
    • Check active jenv and use if present
    • Get the version from .java-version if file present
    • If a local is specified:
      • And no java version present, use local
      • And java version present, check if versions match, i.e. stored version starts with the version from file:
      • If they match, just run
      • If they don't match find matching version and offer to update local config or continue with local config
    • If java version specified use matching java
    • Use global version
FelixSelter commented 2 years ago

@lkleeven Yea that sounds better. You may also want to give a warning if no installed version matches instead of just using the global. After #35 is implemented we should ask the user if he wants to continue with the global or download the appropriate version.How do you plan implementeling the backwards compatibility?

FelixSelter commented 2 years ago

Here is a function to get the Java Version from the executable path https://github.com/FelixSelter/JEnv-for-Windows/blob/main/src/util.psm1#L18

It may be updated to only return the major version This would only affect the automatic naming from jenv autoscan Which would be okay. The major version is easier to remember anyway and most people only care abou it

Use the Open-Prompt utility in the same util file

tech-consortium commented 1 year ago

My recommendation would be to keep the logic as close to how jenv works on linux and macos as much as possible (https://www.jenv.be/). I think you'll get a lot more people using jenv for Windows if that is the case. I'd also recommend you publishing it to some of the popular package managers such as Choco since one hasn't been published to-date.

lkleeven commented 1 year ago

Unfortunately due to a move from a windows machine to mac, partially due to the fact that tools didn't align between me and the other developers who were all on Mac/Linux, I can't really work on this anymore.

In addition I fully agree with the recommendation of trying to keep the tools in line with how they work on different systems. That could have actually prevented my move to Mac.

FelixSelter commented 1 year ago

Thanks for the feedback guys