WorldEdit

WorldEdit

43M Downloads

Regular Expression in WorldEdit.getBlockPattern() unnecessarily complex

LadyCailinBot opened this issue ยท 4 comments

commented

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.

commented

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]*)?%.*
```.
commented

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.

commented

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.

commented

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 ;-)