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 url version #4778

Merged
merged 5 commits into from Feb 23, 2021
Merged

Fix url version #4778

merged 5 commits into from Feb 23, 2021

Conversation

Krzmbrzl
Copy link
Member

This PR contains multiple fixes and refactorings related to the version handling
for versions specified in URLs.

See the individual commits for details

Fixes #4684

Checks

@Krzmbrzl Krzmbrzl added client bug A bug (error) in the software labels Feb 21, 2021
src/mumble/MainWindow.cpp Show resolved Hide resolved
@Kissaki
Copy link
Member

Kissaki commented Feb 22, 2021

As my concern was clarified as not a change by this PR, feel free to ignore my rejection then. I will not be able to do an actual review at least until the end of the week.

When using URLs that don't explicitly state the version, the comment in
code said that in this case we'd assume the version of the currently
running Mumble instance. The code however hardcoded 1.2.0.

This commit fixes this by doing what the commit suggest.
Before this commit we did not report to the user if the version that was
specified in the used URL was invalid.

This commit makes sure that the user is notified about an invalid
version format.
The if condition for checking a URL's version was barely readable.
Therefore the condition was now split into two boolean flags that have
proper names and are computed beforehand.
7a96dc4 adapted the used RegEx to
include support for build-numbers. This however was done without keeping
the option for not specifying a build number (as done for the versions
used in Mumble URLs for instance).

This commit adapts the RegEx such that the build number is now optional.

Fixes mumble-voip#4684
Scanning directory './src'...
Scanning directory './src/mumble'...
Updating 'src/mumble/mumble_en.ts'...
    Found 1958 source text(s) (1 new and 1957 already existing)
@Krzmbrzl Krzmbrzl merged commit 4adc31c into mumble-voip:master Feb 23, 2021
@Krzmbrzl Krzmbrzl deleted the fix-url-version branch November 9, 2022 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug (error) in the software client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mumble url does not work
3 participants