-
-
Notifications
You must be signed in to change notification settings - Fork 405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
shlex for plugin commands #1387
Comments
In my opinion,
It's fast, it's easy to understand, and it works quite well! Except for some cases, where quotes are involved:
It doesn't know you want to use an uncut string in this case, and the user would have to do this :
As you mentioned, I think it could be optional. So it's a 👍 overall! |
I had time to think about this, and I've mixed feeling. On some level, it would be nice to have a more strict command triggering, based on proper detection of arguments. It would be nice, too, if a However, what prevents developers from using (or they are just very lazy, but I don't care for that) So, yeah: I think we should decline this feature request, and improve the documentation. |
Whether an update to documentation or a new attribute on |
As I mentioned on IRC, I don't want to work on subcommands and command arguments until we drop Py2.7. In particular, I'd like to use some Py3+ specific syntax to achieve smooth transitions. Also, it's a complicated topic, and I'm not sure I want to risk that in an already packed with stuff Sopel 7.1 version. |
I didn't even notice this was still milestoned 7.1 lol |
Clearing milestone, as I'm now unsure what advantage we could provide over having the plugin code handle parsing arguments from |
Enough modules define a structured set of subcommands and positional arguments that it might make the code a bit more streamlined to support giving plugins a parsed version of the command args. (Think
admin.py
and itsset
command, the.blocks
suite, and so on.)This is in concept phase for now, but I wanted to follow up my tangential reference to this idea at #1385 with a bit more detail. I reserve the right to bump it further into the future from its starting point as part of Sopel 7, or even drop the idea entirely if it proves to be impractical.
The reasoning is, Sopel's built-in capture groups only handle up to four space-separated arguments (
trigger.group(3)
throughtrigger.group(6)
). For modules to handle more than that, or to handle arguments containing spaces, they have to jump through some annoying hoops withsplit()
—especially to handle arguments that can contain whitespace. Sopel's implementation doesn't even allow specifying "this and all following tokens", which would make the whitespace case more bearable at least.I propose that
trigger
gain a new property (name TBD, but probablyargv
or something similar) containing a tokenized version of the command's arguments, the "arguments" beingtrigger.group(2)
. Something likeshlex
would be helpful for that, as it handles Shell-like syntax automatically (including quoted strings and such).To reduce the performance impact, this could be introduced with a decorator (say,
@sopel.module.parse_args
) and left empty otherwise.As yet, I'm not sure if full-on
argparse
functionality would be useful (or practical to implement in Sopel's module API), but it's a possibility.The text was updated successfully, but these errors were encountered: