string.Trim, TrimLeft, TrimRight optimizations #717

Merged
merged 2 commits into from Aug 8, 2014

Projects

None yet

4 participants

@Divran
Contributor
Divran commented Jun 2, 2014

And removed a few stray characters in string.Explode & Replace

@Divran Divran string.Trim, TrimLeft, TrimRight optimizations
And removed a few stray characters in string.Explode & Replace
830f87f
@robotboy655
Collaborator

The outputs are not identical.

Original:

] lua_run print(string.Trim("HELLO%","%"))
> print(string.Trim("HELLO%","%"))...
HELLO%
] lua_run print(string.Trim("HELLO%","%%"))
> print(string.Trim("HELLO%","%%"))...
HELLO
] lua_run print(string.TrimRight("HELLO%","%"))
> print(string.TrimRight("HELLO%","%"))...
HELLO
] lua_run print(string.TrimRight("HELLO%","%%"))
> print(string.TrimRight("HELLO%","%%"))...
HELLO%

Yours:

] lua_run print(string.Trim("HELLO%","%"))
> print(string.Trim("HELLO%","%"))...
nil
] lua_run print(string.Trim("HELLO%","%%"))
> print(string.Trim("HELLO%","%%"))...
HELLO
] lua_run print(string.TrimRight("HELLO%","%"))
> print(string.TrimRight("HELLO%","%"))...
nil
] lua_run print(string.TrimRight("HELLO%","%%"))
> print(string.TrimRight("HELLO%","%%"))...
HELLO

The second argument needs to be escaped. ( Thanks @wiox for helping me with this )

@Divran
Contributor
Divran commented Aug 8, 2014

The previous string.Trim didn't escape the second argument, though. And the string.Trim before that did. So there was a change there. This means that if I escape the second argument, my string.Trim function still won't behave the same as the current one, but it will behave the same as the one before that.

@willox
Collaborator
willox commented Aug 8, 2014

They should probably all escape the input as they've never been documented to take patterns.

@Divran
Contributor
Divran commented Aug 8, 2014

I just noticed that both string.Trim and my updates completely stop working (or act strangely) if char is longer than one character. I don't think there is any way to properly match multiple characters this way in lua patterns. Should we just say that it doesn't work if char is longer than one character? The documentation has never said it can be, as far as I know.

@Divran
Contributor
Divran commented Aug 8, 2014

Here's the update for escaping input, but that doesn't fix the problem I just mentioned.

@Bo98
Contributor
Bo98 commented Aug 8, 2014

The comments before the functions do refer to char as a single character so I'd say that this should be expected behaviour.

@Divran
Contributor
Divran commented Aug 8, 2014

Yep

@robotboy655 robotboy655 merged commit e562b1b into garrynewman:master Aug 8, 2014
@Divran Divran deleted the Divran:patch-1 branch Aug 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment