noisetorch / NoiseTorch

Real-time microphone noise suppression on Linux.
Other
9.26k stars 229 forks source link

Handle missing patch version by making it optional in regex matching. #417

Closed rajkumarGosavi closed 12 months ago

rajkumarGosavi commented 1 year ago

Given that the server version is not a proper major.minor.patch format, example: 16.0

then the version parsing fails.

To avoid this, Added a fallback version regex, so that if the actual regex fails then we check with the fallback version regex, further since the server version does not contain the patch number setting the patch number to 0 by default.

Something I found while going through the pulseaudio repo, as per my understanding they might be creating major.minor style tags for dev or testing. https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/blob/master/git-version-gen?ref_type=heads

Closes #339

AXDOOMER commented 1 year ago

I'm sure it could be done using a single regex. If the last match group is empty, then use 0 instead for "patch" rather than doing another check using another regex which is essentially the same. No need for two regexes.

rajkumarGosavi commented 1 year ago

@AXDOOMER Here we matching a pattern \.(\d+) before matching .*?, to handle it using a single regex pattern

we can do something like https://go.dev/play/p/pIFHrZlNwPN

diff: .*?(\d+)\.(\d+)\.(\d+).*? .*?(\d+)\.(\d+)\.?(\d+)?.*?

Making the last . and digits optional by using ?.

Let me know If this works.

AXDOOMER commented 12 months ago

Yes, I think that's how you'd do it.

The result of versionRegex.FindStringSubmatch will always be 4 and if the matchgroup for the minor or patch number are empty, they should be set to 0. If the match for the major number is empty, then there's an error because this is totally unexpected.

AXDOOMER commented 12 months ago

Thanks @rajkumarGosavi

Please move the res[3] == "" bloc just above major, err = strconv.Atoi(res[1]) so that Atoi conversions are still all grouped together, then I think the fix is complete.

rajkumarGosavi commented 12 months ago

@AXDOOMER I have implemented the changes.

Thank you.