Open nmlorg opened 5 years ago
One quick thought: If field
, text
, and even desc
become attributes/connected to msg
, this whole path-prefix-peeling process could be reused throughout the UI, not just within /admin
(and /mod
).
Another idea: New adminui.Menu
. If uifunc
is an instance of Menu
, act as if it were a callable that returned it. (Or give it a __call__
with the uifunc
signature.) Instances would include action
to go into msg.action
and something like desc
(but named to not clash with the uifunc
parameter) to go into msg.add
whenever that menu is the final one (rather than just being part of the route). The final attribute could be routes
or fields
(as in adminui.fields
).
Custom invalid_msg
to display if a user types (or selects from a stale menu) a value not listed in fields
, or just use something generic, or just silently ignore? (Theoretically a user could select something from a menu which menu itself is no longer accessible, like if they'd previously gone into a group's calendars list and the group itself is removed/they're removed from its admin list, so displaying "Invalid group XXX" in response to selecting a calendar might be awkward.)
Also, I'm currently planning on implementing this in admin.default
(probably renamed admin_dispatcher
or something) but planning on eventually migrating this to the bot's top-level dispatcher—so all of all modules' entry points will wind up returning a Menu
(instead of non-False
) to signal they've fully dispatched the update(?).
One tricky issue is that right now the adminui
style passes the current frame's parent's conf (which for MODULE.admin is botconf), so for example:
def admin(ctx, msg, modconf, text):
use(modconf[...])
becomes:
def admin(ctx, msg, botconf, field, desc, text):
modconf = botconf[field]
use(modconf[...])
with a slightly awkward transition from moderator.moddispatch
needing to take a step from the modconf
passed to moddispatch
back to the parent botconf
to pass to moderator.admin
—which will then immediately step back into modconf
.
Another issue with the simplest approach (literally having admin.default use adminui.fields) is the change in action from "Choose a module" (etc.) to the hard-coded "Choose a field".
I keep referring to the current location within botconf as the "frame", it might make sense to actually create a new structure called Frame (versus overloading "context") passed through the call signature to keep track of everything.
The simplest gain would be to store parentconf
and field
in Frame, then use getters and setters to allow transparent access to the actual current value (parentconf[field]
) including reassignment.
It could also store both desc
and text
, and include a standardized interface to field, _, text = text.partition(' '); text = text.lstrip()
. (The latter could actually be done during init, with all of next_word
, remainder
, and [full]text
being available for both path component consumers (consuming next_word
and passing remainder
to Frame()
, and for end points that ignore the pre-partitioned text and just use text
.)
(FWIW, I don't think ctx
or msg
would go inside Frame
unless potentiallyFrame
took over responsibility for doing msg.path(field)
.)
That all is probably a good first iteration, then followups might go ahead and move adminui.fields
into Frame
(maybe frame.enter(fields)
).
And for now this will likely be instantiated in admin.default
(and moderator.moddispatch
), but the long-term plan is still to unify all dispatchers, meaning this would end up being moved into MultiBot
's top-level dispatcher.
adminui.log(msg, field, current, new)
If isinstance(new, (list, tuple))
, treat current == None
as current == ()
and show the fields added/removed. Otherwise, do the 4-possibility logic in adminui.integer
(and currently incorrectly implemented in adminui.freeform
), etc., to report the change (directly via msg.add
, or possibly by returning the formatted string, but keep in mind msg.add
escapes HTML when handling format strings directly).
Initially, just use this in place of current redundant logging throughout adminui (and MODULE.admin?), but eventually either somehow hook this into Frame.value.setter
or have adminui.fields
make a [deep? shallow, necessitating changing things like BOTNAME{}.admin{}.admins[]
to BOTNAME{}.admin[]
?] copy of Frame.value
and automatically log on change.
(Alternatively to moving admin.admins
to admin
, we could have modules.admin.admin
return a field set that just includes admins
, so admins
would be Frame.value
when it's updated.)
Menu
Menu.__call__
Frame.next_field
is set:
Frame.next_field
is a name
in Menu.fields
, call its handler(frame.subframe())
.Menu.invalid_field
.msg.action
is not set, call Menu.default(frame)
.Both Menu.fields
and Menu.default
can be callable (the latter probably being the most common), in which case it is called (and, for fields
, its return value is used). This will allow admin.admin
to become a static Menu
with a fields
that iterates over the bot list and a default
that reads config/admin_motd
. Every entry in the top-level admin.admin
Menu
will have a handler of something like admin._ModMenu
, which will be another static Menu
with a static default
and another callable fields
that iterates over the modules list (whose handlers will be their respective MODULE.admin
functions/static Menus).
New plan for Menu
: Hold just fields
, then expose Menu.dispatch
and Menu.display
. All menu-based UIs will be changed to the form:
menu = adminui.Menu(...)
if menu.dispatch(ctx, msg, frame):
return
msg.add('...')
with open('file') as f:
msg.add(f.read())
...
menu.display(ctx, msg, frame)
Going a step further:
menu = adminui.Menu(...)
frame, handler = menu.select(ctx, msg, frame)
if handler: # frame is now the subframe.
return handler(ctx, msg, frame)
msg.add(...)
...
menu.display(ctx, msg, frame)
which will allow things like admin.admin
to not be split:
menu = adminui.Menu(... bot list)
frame, handler = menu.select(ctx, msg, frame)
if not handler:
msg.add(...)
return menu.display(ctx, msg, frame)
ctx.targetbotuser = frame.field
...
# handler is ignored (can just be True in the first Menu).
menu = adminui.Menu(... module list)
if not menu.dispatch(ctx, msg, frame):
msg.add(... module info)
menu.display(ctx, msg, frame)
where menu.dispatch
is just essentially:
def dispatch(self, ctx, msg, frame):
frame, handler = self.select(ctx, msg, frame)
if handler:
msg.path(frame.field)
handler(ctx, msg, frame)
if msg.action:
return True
msg.pathpop()
metabot.modules.countdown.admin exposes an interesting problem: A user enters countdown
, then types a new command name, which enters a new frame, then types a timestamp, and right now steps back two frames to the countdown
menu. The simple pattern of:
menu = adminui.Menu()
now = time.time()
for command, timestamp in sorted(frame.value.items()):
menu.add(command, desc=format_delta(timestamp - now))
frame, handler = menu.select(ctx, msg, frame, create=True)
if not handler:
msg.action = 'Choose a command'
msg.add(
"Type the name of a command to add (like <code>days</code>\u2014don't include a slash "
'at the beginning!), or select an existing countdown to remove.')
return menu.display(ctx, msg, frame, 'command')
would mean the countdown
menu is only displayed if there is nothing in frame.text
, which won't be the case during assignment (at that point it'll be 'commandname timestamp'
). The way it implements it right now is to parse the text out and, if it's complete (including both the command and a valid timestamp), it blanks out the text:
command, _, timestamp = frame.text.partition(' ')
command = command.lower()
if command and timestamp:
if timestamp.isdigit():
...
modconf[command] = timestamp
command = timestamp = None
echo
gets around this because it introduces a second menu layer between the command selection and value assignment, so after you type commandname
you have to select text
, then type your value, dumping you back just one frame at the commandname
menu again (instead of stepping back all the way to the echo
menu). We could do the same thing in countdown.admin
, initially having each commandname
frame be a menu with a single timestamp
field, so after you type the value you'd be back one level at the commandname
frame; or we could have countdown.admin
just set a msg.action (keeping the user in the assignment frame) after providing what would otherwise be a terminal timestamp value. (Either way, the user would have to explicitly click Back after typing their timestamp—either to step back from the 1-entry commandname
menu or to step back from the "Type the time for /commandname" prompt.)
There's definitely a way to do this, like:
newframe, handler = menu.select(ctx, msg, frame, create=True)
if handler:
if newframe.text.isdigit():
adminui.set_log(msg, newframe, int(newframe.text))
elif newframe.text in ('-', ...):
adminui.set_log(msg, newframe, None)
else:
if newframe.text:
msg.add("Can't ...")
return msg.add('This is a little technical ...')
msg.add('Type the name of ...')
menu.display(ctx, msg, frame, 'command')
however, going back to a different alternative, it might be nice to add a desc
field alongside timestamp
(the current flat value) just for documentation purposes, which would introduce the same intermediate menu as echo
has.
There's definitely a way to do this, like:
Wee, this works except that the newly created command doesn't show up in menu.fields
, meaning we'll need to explicitly menu.add
it.
E AssertionError: assert '[chat_id=100...lestestbot]\n' == '[chat_id=1000...lestestbot]\n'
E [chat_id=1000 disable_web_page_preview=True parse_mode=HTML]
E Bot Admin › modulestestbot › countdown: <b>Choose a command</b>
E
E - Set <code>countdowntest</code> to <code>1534906800</code>.
E + /countdowntest is now counting down to <code>1534906800</code>.
E
E Type the name of a command to add (like <code>days</code>—don't include a slash at the beginning!), or select an existing countdown to remove.
E + [/countdowntest (1534906800) | /admin modulestestbot countdown countdowntest remove]
E [Back | /admin modulestestbot]
This also exposes the general issue that whenever menu.fields
changes (either by having a field added/removed during processing, or by having its desc
change), that change does not get picked up automatically if/when we call menu.display
. The latter (desc
changing) might suggest we should have desc
be a callable, to both skip calculation if it's unnecessary (if we enter a msg.action
-setting handler
) and defer it (if it is necessary) until any handlers/local logic have made changes to the data. The former (fields being added/removed), however, goes back to suggesting that fields
itself should be callable, and/or some kind of reusable generator.
Somewhat continuing from c4a74ce and 8ba1c4b, redo the
/admin
system (MODULE.admin and adminui.FIELDTYPE) to be totally uniform.metabot.modules.admin.default
will likely call directly intometabot.util.adminui.fields
rather than handling both bot username selection and module selection in custom logic before directly calling MODULE.admin. All of the various MODULE.admin functions (likemetabot.modules.admin.admin
itself) will be changed from amoddispatch
-style signature (ctx, msg, modconf
) to anadminui
-style signature (ctx, msg, subconf, field, fielddesc, text
). Rather than calling adminui.fields explicitly, all MODULE.admin and adminui.FIELDTYPE functions will return thefieldset
parameter (calledfields
in most current consumers). If instead it returnsNone
, the dispatcher (either adminui.fields itself or admin.default) will assume some action was finalized and use its parent'sfieldset
instead.UI-wise, I think this should be 100% invisible. Code-maintainability-wise, this should be a significant short-term improvement and also a great step toward #23-464266888 ("We could replace
module.admin()
withmodule.SCHEMA
").