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

Ignore duplicate spaces between command args #485

Merged
merged 2 commits into from May 29, 2019
Merged

Ignore duplicate spaces between command args #485

merged 2 commits into from May 29, 2019

Conversation

noahm
Copy link
Contributor

@noahm noahm commented May 10, 2019

In my experience users often add multiple spaces between commands, especially after an @mention due to the way discord "helpfully" autocomplets a space all on its own. e.g.

@botname  hello

Unfortunately, the current command client naively parses the commands into ["", "hello"] and gives up when the empty string isn't a registered command.

This simply filters out any empty strings from the args to allow such messages to be processed as a user would expect.

@LJNeon
Copy link
Contributor

LJNeon commented May 10, 2019

I would +1, but your implementation is lacking. split(/\s+/g) is up to 3x faster and always at least 25% faster, and (imo) much cleaner and better code than filter(s => !!s).

@noahm
Copy link
Contributor Author

noahm commented May 10, 2019

Fair enough, but leading and trailing spaces still add empty strings to the array in that case. How about .trim().split(/\s+/g) ?

@LJNeon
Copy link
Contributor

LJNeon commented May 10, 2019

AFAIK Discord trims leading and trailing whitespace.

@noahm
Copy link
Contributor Author

noahm commented May 10, 2019

It does, but we're not dealing with the entire message at this point. The command client has already stripped off the leading @mention of the bot user plus one space with the substring call immediately preceding the split call.

@LJNeon
Copy link
Contributor

LJNeon commented May 10, 2019

Oh yea you're right, adding trim to handle that should be fine I think.

@ak-s
Copy link
Contributor

ak-s commented May 13, 2019

Not quite on topic, but how about multiline messages? Shouldn't we also split by new line [\s\n]+?

@noahm
Copy link
Contributor Author

noahm commented May 14, 2019

Definitely an option, but I don't see any users sending multi-line messages to a bot and expecting them to work. My motivations here are purely to just remove a papercut I've personally seen happening time and time again.

@abalabahaha
Copy link
Owner

Also to note that the official Discord client trims whitespace, but I've seen other things (bots, unofficial clients) send padded messages

@abalabahaha abalabahaha merged commit ae2fac1 into abalabahaha:dev May 29, 2019
@noahm noahm deleted the patch-1 branch July 17, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants