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

Allows command arguments to consist of only 'escaped' characters #994

Merged
merged 2 commits into from
Jun 5, 2019
Merged

Conversation

Quahu
Copy link
Contributor

@Quahu Quahu commented Mar 24, 2018

Say, we have a command that looks like this:

[Command("test")]
public Task TestAsync(string parameter)
    => ReplyAsync(parameter);

Our prefix will be an exclamation mark.
Invoking it as !test :x: will work perfectly fine, but invoking it as !test \:x: will fail, returning a ParseResult with the error message: The input text has too few parameters..
Also, invoking commands this way with params T[] parameters will invoke the actual command, but with an empty array.

I tested it after my changes, and it seemed to work as expected.

I hope I don't break the lib with this.

@Joe4evr
Copy link
Contributor

Joe4evr commented Mar 24, 2018

How is this different from using [Remainder]?

@AntiTcb
Copy link
Collaborator

AntiTcb commented Mar 24, 2018

Well, you could have parameters after it for one thing.

@Quahu
Copy link
Contributor Author

Quahu commented Mar 24, 2018

@Joe4evr RemainderAttribute wouldn't prevent it from breaking. It would still return the ParseResult in the case I mentioned.

Remainder parameters weren't parsed properly.
@Quahu Quahu changed the title Allows command arguments to only consist of 'escaped' characters Allows command arguments to consist of only 'escaped' characters Apr 4, 2018
@foxbot foxbot requested a review from FiniteReality May 26, 2018 18:15
Copy link
Member

@FiniteReality FiniteReality left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason why we couldn't move the remainder param logic above the escape logic rather than move the escape logic to after handling the default state for the parser?

@Quahu
Copy link
Contributor Author

Quahu commented May 27, 2018

Erm, if I understood your question correctly - it wouldn't fix the characters being escaped. It's not an issue with remainder parameters, but normal parameters.

Copy link
Member

@foxbot foxbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@foxbot foxbot merged commit 7457847 into discord-net:dev Jun 5, 2019
foxbot added a commit that referenced this pull request Jun 5, 2019
Co-authored-by: Quahu <quahu@gmail.com>
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.

None yet

5 participants