Regular Expression in WorldEdit.getBlockPattern() unnecessarily complex
LadyCailinBot opened this issue ยท 4 comments
WORLDEDIT-2707 - Reported by Phoenix_IV
See: https://github.com/sk89q/worldedit/blob/master/src/main/java/com/sk89q/worldedit/WorldEdit.java#L674
It is:
{{s.matches("[0-9]+(?:\.(?:[0-9]+)?)?%.*")}}
Suggestion:
{{s.matches("[0-9]+(\.[0-9]?)?%.*")}}
I just came across this because I had to utilize it. It's like nothing but I still was confused.
At least this part here {{"[0-9]+(?:\.}}{color:red}{{(?:[0-9]+)?)}}{color}{{?%.*"}} makes no sense.
Comment by sk89q
Your regex no longer matches 3.54%block, but ```
([0-9]+)?
The better pattern would be ```
[0-9]+(?:\.[0-9]*)?%.*
```, and with capturing, ```
[0-9]+(\.[0-9]*)?%.*
```.
Comment by Phoenix_IV
Good that you noticed it, I accidentally made use of the question mark instead of the ().
(+) could be used instead of () as well, but it makes no difference for parsing the double.
Now that you've mentioned the capturing again I have a question about it:
Why specifying capturing statements at all? I've removed them because I thought they are unnecessary in a simple .matches() check-up.
Comment by sk89q
I have no idea. It's unnecessary in this case.
Edit: Err, to clarify, it's by default capturing. The ?: makes it non-capturing.
Comment by Phoenix_IV
I know, but what I meant is that I've thought .matches() doesn't support capturing anyway, as it only returns a boolean indicating if the string matches or not. Unlike Pattern.compile(), where a do-not-capture statement would make sense.
Edit: Just noticed that this is fixed now and you removed the capturing as well, so I assume we were talking about the same thing ;-)