version-fox / vfox

A cross-platform and extendable version manager with support for Java, Node.js, Flutter, .Net & more
https://vfox.lhan.me
Apache License 2.0
2.85k stars 102 forks source link

[BUG]: prefix match is not quite correct. #229

Closed aooohan closed 6 months ago

aooohan commented 6 months ago

The fix is not quite correct; if the following versions are available:

golang 1.22.2
golang 1.2.1

vfox use golang@1.2 will match 1.22.2.

Originally posted by @Chance-fyi in https://github.com/version-fox/vfox/issues/224#issuecomment-2071204804

zhylmzr commented 6 months ago

https://github.com/version-fox/vfox/blob/c4b7a5cfb24aacaa22459d7af216d188a0b11c16/internal/sdk.go#L256

Should the matching version numbers be sorted from smallest to largest in terms of semantics, rather than the current largest to smallest sorting?

aooohan commented 6 months ago

Should the matching version numbers be sorted from smallest to largest in terms of semantics, rather than the current largest to smallest sorting?

Descending order is right.

I think there are two issues here:

  1. We should not do prefix match logical if the version is specified. This can cause minor performance issues.
  2. The version number format maybe take several forms, it is also unreasonable to match by adding ., eg: 21.0.3+9-zulu
zhylmzr commented 6 months ago
  1. Only when the plugin does not implement PreUse will it do a prefix match, but either way, it's a trade-off.
  2. VersionSort sorts version numbers by dividing them with ., and can be extended to divide version numbers with [.+-].
aooohan commented 6 months ago
  1. Only when the plugin does not implement PreUse will it do a prefix match, but either way, it's a trade-off.

Right!

  1. VersionSort sorts version numbers by dividing them with ., and can be extended to divide version numbers with [.+-].

I don't like this, the rules for version numbers are a custom behavior of the plugin, and can contain other messy characters as well. I don't want to be restrictive.

For examples:

https://github.com/yanecc/vfox-crystal

vfox install crystal@dev
vfox install crystal@nightly
vfox install crystal@20240420
zhylmzr commented 6 months ago

VersionSort is currently only used in the use process. Regardless of what version alias is supported in the plugin, its value is only related to the directory name where the sdk is installed. If the plugin installs the SDK to the same directory name using the 'nightly' alias, VersionSort indeed cannot accurately represent the version.

aooohan commented 6 months ago

Regardless of what version alias is supported in the plugin, its value is only related to the directory name where the sdk is installed.

Indeed, I forgot this and thought wrongly.