irc: configure anti-looping system

[Exirel]

Description

Tin, fix #1618.

The feature was surprisingly easy to implement, this is a draft because I want to make sure I've the right names before writing more documentation.

Checklist

• I have read CONTRIBUTING.md
• I can and do license this contribution under the EFLv2
• No issues are reported by make qa (runs make quality and make test)
• I have tested the functionality of the things this change touches

[dgw]

I keep forgetting that people ask about that, even though we do have an issue about it. 😅 Thanks for just sitting down and writing the patch. Few general thoughts:

• How about antiloop_ as a prefix for these options? Though I'm all ears for something that fits a bit better with flood_.
  • As ideas for new names:
    • anti_looping -> antiloop_threshold
    • anti_looping_repeat -> antiloop_silent_after
    • anti_looping_timeframe -> antiloop_window
• With the other options already set, why not add a way to override the ... message? Maybe someone would like Sopel to say "Ditto", for example.

[Exirel]

And I even added tests.

[dgw]

Why didn't I notice the hard-coded lookback number (8 lines) before? No idea. Must have looked at the code on my phone, which always makes it easy to get lost. Small screen problems.

And I even added tests.

Blasphemy! Ban this man! /s

[Exirel]

Albeit not perfect, the time window has it uses as it prevents some false positive. However, it's not really well done, because it only check if the last message is within that time window, so you could have one message 5s away, and the previous 7 a day old, and it'll still think there is a loop.

However, I don't feel like fixing that in this PR, I would like to keep the configuration change and the behavior change as separated as possible.

Why didn't I notice the hard-coded lookback number (8 lines) before?

On the other hand, Sopel remembers 10 messages per channel/user anyway. So I can remove that limit and just look at the last 10 messages directly.

[Exirel]

And so I changed the 8 to 10 and changed the doc accordingly. I added a warning in the doc for good measure, because setting the threshold to more than 10 will deactivate the loop prevention system and that might not be obvious to a user.

[dgw]

Is there a reason not to clamp the value to max 10 with a custom parse (and/or serialize) function? Disabling the flood prevention is documented explicitly (and improved on by one of my recent PRs).

[dgw]

One more implementation thing about the number of messages considered, while I'm awaiting Exi's thoughts on clamping the antiloop_threshold value to max 10.

[Exirel]

Given the status of the review, I squashed the commits while adding the min(10, 'antiloop_threshold'). I replaced the warning by the note, and that's all I did in here.

[dgw]

I squashed the commits while adding the min(10, 'antiloop_threshold'). I replaced the warning by the note, and that's all I did in here.

You also rebased while doing that, though. I can't click "force-pushed" in the event history to get the diff between the previous state and the current state without wading through clutter pulled from upstream. :/

So I re-reviewed everything, and came up with more line notes than I probably would have if I'd only had to look at the new changes. Serves you right. 🤪

[Exirel]

And I applied the changes. No squash, no rebase yet. You tell me when it's all OK for you.

[dgw]

As far as I can see, this is good to go (squash). If I missed anything by reviewing on my phone, so be it. 🙃

[Exirel]

Squashed.