Skip to content
This repository has been archived by the owner on Jul 12, 2020. It is now read-only.

Add string:split() #192

Closed
wants to merge 4 commits into from
Closed

Add string:split() #192

wants to merge 4 commits into from

Conversation

Validark
Copy link
Contributor

@Validark Validark commented Apr 16, 2019

Changes:

  • Adds string.split to the string table so it can be used as a method

Fixes #191

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 98.018% when pulling 1beb4b2 on Validark:patch-1 into a69e245 on LPGhatguy:master.

@LPGhatguy
Copy link
Owner

This causes string.split to leak outside of the actual Roblox environment! Ideally, any changes we make to existing Lua APIs should be isolated to inside the habitat if possible to minimize side effects of Lemur being run.

Instead of assigning it here, we should see if there are any tricks we can use to make callers inside the environment use our fake string table as the fallback table for strings. I know that debug.setmetatable can accomplish this globally, which is not ideal.

@Validark
Copy link
Contributor Author

While I understand your sentiment, this is a highly improbable hypothetical. A user could just as easily overwrite all the string functions before running Lemur, and then get upset when Lemur (and every other library ever which deals with strings at all) breaks. I would point out that this project is for testing Roblox code. In Roblox, you cannot overwrite members of the default math/string namespaces. Nobody using this would ever dream of doing so. We are already assuming we know what byte/sub/find do. At most, a developer could reasonably wrap the original function and make it print something to the console before returning the results of the built-in function. Unless you can think of a hack, or plan on running Lemur in a separate VM, this is really not a big deal and can be easily documented with a one-line heads up. It is more important to have this work than trying to not expose string.split globally. It's perfectly fine to expose it globally in this case.

@Validark Validark closed this Aug 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add string:split()
3 participants