[patch] Dollar character fix, maybe

Notes on testing the latest builds of MapTool

Moderators: dorpond, trevor, Azhrei

Post Reply
tiorthan
Cave Troll
Posts: 84
Joined: Fri Aug 24, 2012 8:56 am
Location: Germany
Contact:

[patch] Dollar character fix, maybe

Post by tiorthan »

There is a little issue with dollar characters in strings in some functions as stated in http://forums.rptools.net/viewtopic.php ... 64#p214964. This two-line change might already fix the issue. I did this as a drive-by fix while examining other things, though, So I'm not really sure if it wouldn't have any side effects.
Attachments
dollarcharacterfix.txt
(673 Bytes) Downloaded 95 times

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: [patch] Dollar character fix, maybe

Post by Azhrei »

Hm. I guess this is a bug. The Wiki: encode() function doesn't sound like it takes a regex as a parameter, in which case it makes sense to escape the dollar sign. Instead of adding a bunch of backslashes we can add "\Q" at the start and "\E" at the end and the regex compiler will do all the real work. Given the way encode() is defined and used (or rather, how undefined it is!), that should work fine. Anyone relying on undocumented behavior will get what they deserve. ;)

(I'm only partially kidding. We obviously want existing code to work and if it does when using \Q\E then that's great. if it doesn't then we'll need to decide what the next step would be.)

Lee
Dragon
Posts: 958
Joined: Wed Oct 19, 2011 2:07 am

Re: [patch] Dollar character fix, maybe

Post by Lee »

wolph encountered an error with the applied patch detailed here.

tiorthan
Cave Troll
Posts: 84
Joined: Fri Aug 24, 2012 8:56 am
Location: Germany
Contact:

Re: [patch] Dollar character fix, maybe

Post by tiorthan »

In the linked example above, the problem occurs while parsing the argument list of json.append(), it's not an encode problem as the function responsible, if I remember correctly, is called to evaluate all function parameter lists.

My patch won't work, and neither will wrapping the string in \Q\E (because that's basically the same), because it inevitably leads to the bug found by wolph. Worse still, the newly introduced bug has no workaround for the user whereas the current bug has.

At the moment I cannot think of any clean solution, with "clean" meaning maintainable and with a low-enough risk of introducing new bugs.

Post Reply

Return to “Testing”