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

[BUG] Incorrect keyword-only argument parsing #29

Closed
nexy7574 opened this issue Oct 20, 2024 · 2 comments · Fixed by #32
Closed

[BUG] Incorrect keyword-only argument parsing #29

nexy7574 opened this issue Oct 20, 2024 · 2 comments · Fixed by #32
Assignees
Labels
bug Something isn't working
Milestone

Comments

@nexy7574
Copy link
Owner

Describe the bug
When providing a *, keyword argument in a command function definition, the library is supposed to make it a "greedy" argument, in that any content unconsumed goes into that variable. Right now, this does not happen.

To Reproduce
Run this simple example:

import niobot

mybot = niobot.NioBot(homeserver="https://example.com", user_id="@example:example.com", command_prefix="!")


@mybot.command()
async def echo(ctx, *, content: str):
    await ctx.respond(content)

Expected behavior
!echo foo bar should respond with foo bar, however, the too many arguments exception is raised

Additional context
Using "quotes" gets around this issue, so the issue may simply just be the command parser being too strict when counting arguments and prematurely raising an error.

@nexy7574 nexy7574 added the bug Something isn't working label Oct 20, 2024
@nexy7574 nexy7574 self-assigned this Oct 20, 2024
@nexy7574 nexy7574 added this to the v1.2.0 milestone Oct 20, 2024
@nexy7574
Copy link
Owner Author

This is now fixed in fix/argument-parsing, but be warned, there has not been extensive testing. There are possible regressions relating to optional arguments, positional arguments, argument typing, and even argument parsing. If anyone tests this out, please let me know how it worked out for you ("works for me" is just as valuable as "it didnt work for me")

@nexy7574
Copy link
Owner Author

nexy7574 commented Nov 4, 2024

Will merge soon - no issues have been reported and I have been running this in a stable fashion for 2 weeks with no issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant