search

sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
951 stars 405 forks source link

adminchannel: empty `.tmask` handled incorrectly #2596

Closed dgw closed 3 months ago

dgw commented 4 months ago

Description

Running .tmask without any argument technically sets an empty topic mask, but in a broken way.

Reproduction steps

  1. Enable adminchannel plugin if not enabled
  2. As chanop, do just .tmask (no argument)
  3. Do .showmask
  4. Sopel responds with an exception: Unexpected TypeError (can only concatenate str (not "NoneType") to str) from dgw. Message was: .showmask

Expected behavior

adminchannel should properly clear the topic mask in this case, such that doing .showmask yields the same output as if .tmask had never been set at all.

Additionally, .showmask should handle None as if the topic mask is unset. Defense in depth!

Relevant logs

No response

Notes

I've a half-assed patch for this in adminchannel-tmask-none branch, but it's untested and probably needs some more work before becoming a pull request.

Sopel version

973a489355540d68b95db01a49e983ac7a740bcc

Installation method

pip install

Python version

3.10.7

Operating system

No response

IRCd

No response

Relevant plugins

adminchannel

half-duplex commented 4 months ago

Unless we have an established pattern, I prefer possibly-destructive things be explicit: an argument of empty quotes, a hyphen, -clear, or similar to clear it, .tmask without arguments shows usage including how to clear.

dgw commented 4 months ago

The thought of adding a .clearmask or .tmclear command did cross my mind, yes. I had forgotten about .showmask, didn't expect .tmask to accept an empty arg either, and was surprised.

dgw commented 3 months ago

Process note: We're likely to slot #2601 into 8.0.0 because it's a user-facing change and not a pure bugfix, so I've changed the issue's target milestone.