CodeCrafter47 / TabOverlayCommon

GNU General Public License v3.0
1 stars 5 forks source link

Add additional operands #14

Closed Andre601 closed 8 months ago

Andre601 commented 8 months ago

This PR - if it is properly made I hope - adds the following new operands to use:

These patterns have been taken from TAB which uses them for these exact purposes. Adding them would be beneficial for BungeeTabListPlus, as it would allow people to do expressions such as ${viewer server} |- "lobby-" to check if the viewer's server name starts with "lobby-", allowing tab lists to become even more dynamic.

I hope this syntax doesn't conflict with the negated number one. If it does could a different character for the - be used (i.e. ~).

CodeCrafter47 commented 8 months ago

This seems nice. The |- and -| won't cause any conflicts, at least none that I see right now. I'd suggest adding natural language token such as starts_with and ends_with in addition to the shortcuts (because I don't think I'll be able to remember which is which).

The <- does cause a conflict, I think. -2<-1 will be parsed as (-2) <- (1) instead of (-2) < (-1). So I think we need a different token for contains. Since you suggested on discord using tilde for case-insensitive variants of various string operations, I'd like to keep that free and look for a different alternative.

Also there's a bunch of tests for the expression engine in DefaultExpressionEngineTest. I suggest you add at least the example I gave above, to make sure it'll continue working as expected.

Andre601 commented 8 months ago

I'd suggest adding natural language token such as starts_with and ends_with in addition to the shortcuts (because I don't think I'll be able to remember which is which).

Do you mean adding a <expression> starts_with/ends_with <expression> like there is and and or for && and || respectively?

Also the only option I can think of right now for the contains token would perhaps be using <_ but this could look strange to be honest... So maybe this one could for now just be a keyword (contains) and a specific symbol/pattern could be used later.

Andre601 commented 8 months ago

Actually, thinking about it, <_ might be a good solution here. Because the _ tells me that there would be something that the left text should have. Will commit this change now, alongside adding word-variants of the operands (starts_with, ends_with and contains).

Might as well add the case-insensitve checks if you're up for it.

Andre601 commented 8 months ago

Something I now wonder is, if there should be operators for negated checks of these new options. Like f.e. |! for "does not start with".

Not sure if that can't already be done with current negation logic implemented here... Like would !(<expression> |- <expression>) work as a "does not start with" condition? If not, then perhaps implementing the negated operators could be useful here.

CodeCrafter47 commented 8 months ago

I have merged the changes. !(<expression> |- <expression>) will be does not start with, so there is no need for a dedicated operator.

If you like, you can also add the case-insensitive checks. I think that will be a valuable addition.

Andre601 commented 8 months ago

Will do so once I'm back home