Added filtering to /jainfo (issue #62)

[waffle-stomper]

Closes #62

The new /jainfo syntax looks like this:

/jainfo [<page number> or 'next'] [censor] [action=<action type>] [player=<username>]

Actions can be specified in LoggedAction format (KILL, BLOCK_PLACE, etc) or the format that's displayed in juke logs (Killed, Block Place, etc). Note that you will need to enclose any actions with a space in double quotes, (e.g. action="Block Place").

Arguments can be in any order and case doesn't matter. The player parameter has wildcards around it, so any substring of a username will match (e.g. player=grief will match dirtygriefer334 and GrIeFiNgNaMeS).

I've also added tab completion, and fixed the next/censor logic. It was a bit weird before.

[CivcraftBot]

Can one of the admins verify this patch? Type 'ok to test' to test.

[Maxopoly]

Did you test this yet?

[ProgrammerDan]

Hmm, might need to rebase this.

[waffle-stomper]

Sorry guys!

@Maxopoly - I've tested it locally and think I've got all of the bugs worked out. I just forgot to mention it.

@ProgrammerDan - I thought that once the other PR was accepted, the first commit would disappear. It seems that I was incorrect. I've rebased the commit for this PR. Hopefully I haven't ruined anything!

For anyone - what's the official position on spaces vs. tabs for indentation? The code looks like it's mostly spaces, but there are blocks of tabs dotted around.

[ProgrammerDan]

We're hoping for a single PR to fix all the spaces to tabs, with no other changes. Functionality changes like this shouldn't fix large areas of formatting.

[waffle-stomper]

That makes a lot of sense. Maybe that'll be my next project. I've reverted the tab conversions, so it should be pure functionality now.

[waffle-stomper]

Thanks for the detailed notes and for wording everything in a nice way. I know it can be frustrating to work with less competent people, so I appreciate the effort.

I've made all of the changes you requested and I think I've tested everything.

I did make two small alterations:

• In JukeAlertLogger, I swapped my dynamic statment generator for four PreparedStatements instead of four Strings that would then be converted into PreparedStatements. I'm happy to change them to Strings if you'd prefer that though.
• Apparently you can't override the valueOf method for an enum, so I named the function that converts a String to a LoggedAction 'fromString'.

One last thing, I noticed this dire warning in LoggedAction: // ONCE THIS GOES INTO PRODUCTION, _NEVER_ CHANGE THESE, only mark some as not used and add more with larger values! I'm reasonably sure they're just talking about not changing the ID values and that adding the extra attributes shouldn't break anything, but I wasn't 100% sure.

[ProgrammerDan]

@waffle-stomper My pleasure to review.

less competent

Not at all -- this is the perfect place to gain new experience, especially if its stuff you haven't done before. Competence you've got, or we wouldn't be having this discussion at all :).

for four PreparedStatements

Don't store the PreparedStatements; go ahead and "instantiate" them when you need them, preferably wrapped in a try-with-resources block. I know that most of the plugins still have the "anti-pattern" of pre-instantiation of all PreparedStatements, but I'm trying to go through and remove those as I have time, so feel free to join in the effort :).

can't override

Yeah, that was my mistake, your solution is perfect.

dire warning

Correct, that has to do with ordering ... In the code somewhere, Ordinals are used (which is a big no-no) iirc, which are specific to the ordering of the elements in the enum, hence the request never to change them; in reality, just don't re-order them. You can augment them without fear :)

[waffle-stomper]

Is there a nicer way to do this? I tried assigning all of the statements up in the block of declarations at the top of the class, but it felt weird. I ended up leaving them in the initializeStatements method, but that meant they couldn't be final.

[ProgrammerDan]

Ah right, because of the format ... due to that slightly odd design decision (not your fault!) I'd say you're constrained to what you have. It's still superior to have the strings as unchanging once configured, whether static final or not just changes which string map Java stores them in, iirc, but either way you're in a better place with these changes.

[Maxopoly]

Seems ready for merge?

[waffle-stomper]

Whoops! It's fixed now.