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

Escaping quotes is required in some places it shouldn't be #590

Closed
Pikachu920 opened this issue May 24, 2017 · 3 comments
Closed

Escaping quotes is required in some places it shouldn't be #590

Pikachu920 opened this issue May 24, 2017 · 3 comments
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: medium Issues that are detrimental to user experience (prohibitive bugs or lack of useful implementation).

Comments

@Pikachu920
Copy link
Member

set {%subtext of "test" from characters 1 to 1%} to "value"

should work, since there shouldn't be a need to double quotes as the expression isn't inside a string, however quotes still need to be escaped in this scenario.

@bensku
Copy link
Member

bensku commented May 25, 2017

Could you test this with older Skript versions? I can figure out where it comes from quicker if it was introduced by JSON chat.

@Pikachu920
Copy link
Member Author

This was on dev25, so it was before json's time

@bensku bensku added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jun 1, 2017
@Snow-Pyon Snow-Pyon added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: medium Issues that are detrimental to user experience (prohibitive bugs or lack of useful implementation). and removed enhancement Feature request, an issue about something that could be improved, or a PR improving something. labels Jan 31, 2018
@TPGamesNL
Copy link
Member

The cause of this issue is is VariableString.newInstance, line 214 and line 223. Since variable 'strings' (aka VariableStrings in StringMode.VARIABLE_NAME, which is used for all stuff between the { and }) are the only StringMode that isn't actually in quotes, it the double quotes shouldn't be dealt with (the check if it only contains double quotes on line 214 and the replacing double quotes with single quotes on line 223). This can be solved by adding simple conditions on the two lines that check if the type isn't VARIABLE_NAME (I tested it, it works).

I do have one concern, which is that it has been 3 and a half years (at least) since this bug has been present, and fixing this could break scripts that rely on this bug. I also don't see a simple way to accept both, as {_%length of "" and ""%} has different (valid) interpretations depending on whether the bug is present:
{_%length of "" and ""%} -> {_%length of ("") and ("")%} -> {_0 and 0} when the bug is not present.
{_%length of "" and ""%} -> {_%length of ("" and "")%} -> {_5} when the bug isn't present.

@TPGamesNL TPGamesNL added the PR available Issues which have a yet-to-be merged PR resolving it label Mar 23, 2021
@TPGamesNL TPGamesNL added completed The issue has been fully resolved and the change will be in the next Skript update. and removed PR available Issues which have a yet-to-be merged PR resolving it labels May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: medium Issues that are detrimental to user experience (prohibitive bugs or lack of useful implementation).
Projects
None yet
Development

No branches or pull requests

5 participants