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 fallback regex for OpenSuse #451

Merged
merged 1 commit into from
May 17, 2019
Merged

Fix fallback regex for OpenSuse #451

merged 1 commit into from
May 17, 2019

Conversation

n-rodriguez
Copy link
Contributor

@n-rodriguez n-rodriguez commented May 15, 2019

See #377

This regex matches both case :

VERSION = 15.0 and
VERSION="15.0"

Thank you!

@tas50
Copy link
Contributor

tas50 commented May 15, 2019

@n-rodriguez Can you add a comment above the regex with a permalink from rubular. https://rubular.com/

That way we can have a link in the code to show the various things we're trying to match instead of having to track things back to this PR.

Also you'll need to sign this commit off for the DCO before we can merge it. https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

@n-rodriguez
Copy link
Contributor Author

@tas50 done! Thank you!

@robbkidd
Copy link

Seems OK with more variants: https://rubular.com/r/b5PN3hZDxa5amV

And an example for the regex appearing above it: https://rubular.com/r/UKaYWolCYFMfp1

Signed-off-by: Nicolas Rodriguez <nicoladmin@free.fr>
@n-rodriguez
Copy link
Contributor Author

@robbkidd done! thank you! 👍

@miah miah requested a review from zenspider May 15, 2019 21:41
@zenspider
Copy link
Contributor

I dunno... should I be the anal-retentive regexp-er? I am not sure it actually matters because this is coming from some vendored source?

I'm gonna approve, but just so you know, that \s accepts things like tabs or newlines. You probably just want <space>?... and the "? allows for the quotes to be on one side and not the other. Again, probably doesn't matter, but in other contexts that sort of thing is crucial.

@n-rodriguez
Copy link
Contributor Author

n-rodriguez commented May 15, 2019

Actually [[:space:]] is an "alias" to \s : https://ruby-doc.org/core-2.6.3/Regexp.html#class-Regexp-label-Character+Classes. It has the same behavior (/[[:space:]]/ - Whitespace character ([:blank:], newline, carriage return, etc.))

@zenspider
Copy link
Contributor

I meant / ?/ (added slashes so you can see the space, vs /\s?/. The first one only accepts a space.

@n-rodriguez
Copy link
Contributor Author

I meant / ?/ (added slashes so you can see the space, vs /\s?/. The first one only accepts a space.

Ok. Do you want me to change it?

@miah miah merged commit b6b3dde into inspec:master May 17, 2019
@zenspider
Copy link
Contributor

I'll get it. Just consider it for next time.

@n-rodriguez
Copy link
Contributor Author

I'll get it. Just consider it for next time.

I'll remember! Thanks!

myii added a commit to myii/template-formula that referenced this pull request May 25, 2019
myii added a commit to myii/postgres-formula that referenced this pull request May 31, 2019
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.

5 participants