Skip to content

Regex Little Changes#1175

Merged
KyleSanderson merged 4 commits intoalliedmodders:masterfrom
TheByKotik:regex
Feb 26, 2020
Merged

Regex Little Changes#1175
KyleSanderson merged 4 commits intoalliedmodders:masterfrom
TheByKotik:regex

Conversation

@TheByKotik
Copy link
Contributor

  • Prevented to triple and double call strlen.
  • More informative message on if (offset >= len).
  • Add missing parametr in navite MatchRegex.

P.S. I'm have little exp in c++, sorry if something wrong.

* Prevented to triple and double call `strlen`.
* More informative message on `if (offset >= len)`.
* Add missing parametr in navite `MatchRegex`.
Copy link
Member

@Headline Headline left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup and missing param discovery!

Just a few comments and this one should be good to go

@Headline Headline added the Bug general bugs; can be anything label Feb 9, 2020
* Using `strdup` instead `strcpy`.
* Replaced NULL to nullptr.
* Removed note about MatchOffset.

Co-Authored-By: Headline <[email protected]>
@TheByKotik TheByKotik requested a review from Headline February 10, 2020 05:44
Copy link
Member

@Headline Headline left a comment

Choose a reason for hiding this comment

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

two more nits and this is good

Co-Authored-By: Headline <[email protected]>
@TheByKotik TheByKotik requested a review from Headline February 10, 2020 06:06
Co-Authored-By: Headline <[email protected]>
@Headline
Copy link
Member

Has this been tested?

@Headline Headline added the Feature Request user requested feature label Feb 10, 2020
@TheByKotik
Copy link
Contributor Author

Has this been tested?

Windows build only with test.sp.

Copy link
Member

@Headline Headline left a comment

Choose a reason for hiding this comment

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

This looks acceptable now.

Regex.Match already supports offsets so I'll leave this to someone else to decide on. Patch started as cleanups, and has since dissolved to the addition of a 3rd parameter to old-style MatchRegex.

@Headline Headline removed the Bug general bugs; can be anything label Feb 11, 2020
@KyleSanderson KyleSanderson merged commit ded3867 into alliedmodders:master Feb 26, 2020
@KyleSanderson
Copy link
Member

Not really a bug but a feature request. Nice hidden API exposure there buddy :-) thanks for the PR and resolving this.

rumblefrog added a commit to rumblefrog/sp-gid that referenced this pull request Feb 27, 2020
TeleportEntity def value: alliedmodders/sourcemod#1167
MatchRegex added offset param: alliedmodders/sourcemod#1175
IsServerProcessing documentation wording: alliedmodders/sourcemod#1188
TR_EnumerateEntities(Sphere/Box/Point): alliedmodders/sourcemod#1145
@TheByKotik TheByKotik deleted the regex branch March 2, 2020 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Request user requested feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants