Skip to content

Conversation

@Exirel
Copy link
Contributor

@Exirel Exirel commented May 28, 2020

Description

Based on #1873

This PR tweaks the previous PR to replace spaces by a - to generate command's label (the display name, not the "name" or the "regex"). It assumes that command name (and aliases) are not regular expression, but simple string, that need re.escape. That's a problem because apparently it wasn't a bug, but a feature.

The thing is, if someone wants to use a subcommand today, they have to escape it manually, or to use a generic pattern that is hard to document, like this:

commands('main\s\w+')
commands('main\ssub')

Which is quite odd. I discovered that problem by accident, and discussed with @dgw. I'll amend #1873 to make the system backward compatible, trying to be smart with it.

Meanwhile, this PR is published to show the tests and the documentation written for subcommands. This is the result of a reverse engineering, and I think Sopel will be better with it.

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

@lgtm-com
Copy link

lgtm-com bot commented May 28, 2020

This pull request introduces 3 alerts when merging 7e398f2 into 7144d57 - view on LGTM.com

new alerts:

  • 3 for Conflicting attributes in base classes

@Exirel
Copy link
Contributor Author

Exirel commented May 28, 2020

I'm merging this into the base PR, because working on this bit makes me discover the "command name can be regular expression" and makes me question the Universe.

So I've added tests and fixed it to re-allow regular expression, but since #1873 is incomplete without this fix, I merge the two together, and close this one.

@Exirel Exirel closed this May 28, 2020
@lgtm-com
Copy link

lgtm-com bot commented May 28, 2020

This pull request introduces 3 alerts when merging 8d32478 into 7144d57 - view on LGTM.com

new alerts:

  • 3 for Conflicting attributes in base classes

@Exirel Exirel deleted the core-command-subcommand branch June 23, 2020 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant