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

Split version number with regex #16

Merged
merged 1 commit into from
Apr 5, 2021
Merged

Split version number with regex #16

merged 1 commit into from
Apr 5, 2021

Conversation

yol
Copy link
Member

@yol yol commented Feb 28, 2021

Third time's the charm!

This is so that the same regex can be used in the add-on checker.
It was tested to run fine with all current add-ons (except the special
case pvr.waipu, which the author was asked to change).
@yol yol requested a review from wsnipex February 28, 2021 19:51
@yol yol self-assigned this Feb 28, 2021
parts = result[0].rsplit("-", 1)
if parts and re.match(r"^[\w.\-@]+$", parts[0]) and re.match(r"^\d+\.\d+(\.\d+){0,4}([+~\w.]+)?$", parts[1]):
return tuple(parts)
m = re.match(r"^([\w\.]+.*)-(\d+\.\d+(\.\d+){0,4}([+~\w]+(\.\d+)?)?)$", result[0])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be more strict with package names and not allow .*?

@wsnipex
Copy link
Member

wsnipex commented Feb 28, 2021

I wouldn't. We've tried the sticter approaches and failed :P

@yol
Copy link
Member Author

yol commented Mar 7, 2021

The regex that is there now ([\w.\-@]+) did match all the packages

@yol
Copy link
Member Author

yol commented Apr 5, 2021

I guess it's fine if just the add-on checker uses the stricter variant @enen92

@yol yol merged commit 5c9acaf into xbmc:master Apr 5, 2021
@yol yol deleted the fix_parsing branch April 5, 2021 06:28
@enen92
Copy link
Member

enen92 commented Apr 5, 2021

Maybe it's better to add a check for the version instead of the regex, that way we have a bit more freedom to check whatever we see fit. I'll try to have a look after work

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.

4 participants