Skip to content
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

Open
dgw opened this issue Sep 26, 2018 · 6 comments
Open

shlex for plugin commands #1387

dgw opened this issue Sep 26, 2018 · 6 comments

Comments

@dgw
Copy link
Member

dgw commented Sep 26, 2018

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 its set 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) through trigger.group(6)). For modules to handle more than that, or to handle arguments containing spaces, they have to jump through some annoying hoops with split()—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 probably argv or something similar) containing a tokenized version of the command's arguments, the "arguments" being trigger.group(2). Something like shlex 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.

@dgw dgw added this to the 7.0.0 milestone Sep 26, 2018
@Exirel
Copy link
Contributor

Exirel commented Dec 26, 2018

In my opinion, shlex has many advantages:

>>> shlex.split('.command arg1 "arg 2" \'arg3\' arg4')
['.command', 'arg1', 'arg 2', 'arg3', 'arg4']

It's fast, it's easy to understand, and it works quite well!

Except for some cases, where quotes are involved:

>>> shlex.split('.command I want to "quote something"')
['.command', 'I', 'want', 'to', 'quote something']

It doesn't know you want to use an uncut string in this case, and the user would have to do this :

[nick] .command 'I want to "quote something"'

>>> shlex.split('.command \'I want to "quote something"\'')
['.command', 'I want to "quote something"']

As you mentioned, I think it could be optional. So it's a 👍 overall!

@Exirel
Copy link
Contributor

Exirel commented May 1, 2019

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 Trigger object would give out its argument as if it was a shell command line.

However, what prevents developers from using shlex themselves? Maybe they don't know about it, so we should write more documentation with usage example of that. The more I think about it, the more a "common pattern for Sopel plugins" sound like a good candidate for a "How To" page somewhere in Sopel's documentation.

(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.

@dgw dgw modified the milestones: 7.0.0, 7.1.0 Nov 16, 2019
@dgw dgw changed the title argparse/shlex for module commands shlex for plugin commands Nov 16, 2019
@dgw
Copy link
Member Author

dgw commented Nov 16, 2019

Whether an update to documentation or a new attribute on trigger with pre-shlexed arguments (what I was thinking), this just isn't important enough for 7.0. Punted.

@Exirel
Copy link
Contributor

Exirel commented Oct 2, 2020

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.

@dgw
Copy link
Member Author

dgw commented Oct 2, 2020

I didn't even notice this was still milestoned 7.1 lol

@dgw dgw modified the milestones: 7.1.0, 8.0.0 Oct 2, 2020
@dgw dgw removed this from the 8.0.0 milestone Jul 14, 2022
@dgw
Copy link
Member Author

dgw commented Jul 14, 2022

Clearing milestone, as I'm now unsure what advantage we could provide over having the plugin code handle parsing arguments from trigger.group(2) on its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants