Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/Builder #32

Closed
wants to merge 3 commits into from
Closed

Fix/Builder #32

wants to merge 3 commits into from

Conversation

shovelmn12
Copy link
Contributor

Fix/Builder

@shovelmn12 shovelmn12 closed this Jan 14, 2025
@shovelmn12 shovelmn12 reopened this Jan 14, 2025
@MohiuddinM
Copy link
Owner

Thanks for the contribution I have fixed the problem

@MohiuddinM MohiuddinM closed this Jan 14, 2025
@shovelmn12
Copy link
Contributor Author

FYI

      final version = Platform.version.substring(0, 5);

This wont work when one of the numbers will be a double digit ...
Thats why I used a regex

@MohiuddinM
Copy link
Owner

This wont work when one of the numbers will be a double digit ... Thats why I used a regex

Thanks for the info, I fixed it for double digits too. I didn't use this PR because regex seems like an overkill here, I hope you understand.

@shovelmn12
Copy link
Contributor Author

shovelmn12 commented Jan 15, 2025

No worries

      final version = versionText.substring(
        0,
        versionText.indexOf(' '),
      );

The only issue I can see with that is what if one of the platforms implements Platform.version as version number only (ex. Platform.version = "3.6.0" parsing it will fail, there is no space to find ...

add a fallback at least, something like versionText.indexOfOrNull(' ') so in case there is no space it defaults to the whole string.

Thats why I used RegExp as we donnow how the underlying platform implements Platform.version so I needed to ensure some kind of \d+.\d+.\d+ exists ... because the solution using indexOfOrNull will fail when Platform.version = "" for example or if Version.parse does not know how to handle "3.6" and Platform.version = "3.6"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants