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

Fix prefix parsing #81

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix prefix parsing #81

wants to merge 3 commits into from

Conversation

emersonveenstra
Copy link

The regex matcher tried to specify specific characters for a nick,
which doesn't work anymore, especially with things like RELAYMSG

Signed off by Emerson Veenstra [email protected]

The regex matcher tried to specify specific characters for a nick,
which doesn't work anymore.

Signed off by Emerson Veenstra <[email protected]>
@emersonveenstra
Copy link
Author

I guess i need to fix some tests 😬

@tadzik
Copy link

tadzik commented Aug 5, 2021

The regex matcher tried to specify specific characters for a nick,

Right, it's currently (on master) not very future proof. Have you considered using a broader character class instead, similarly to how the latter part of the current regex does, something like /^([^!]*)(!([^@]+)@(.*))?$/? Not sure if that's more readable in practice (shame that JS regexes don't have an /x modifier), but would save us looking at the code and trying to figure out if it doesn't have an accidental off-by-one ;)

In any case, can we get some tests for the // I don't think we'll get here bit, just in case? :)

aabluedragon added a commit to aabluedragon/node-irc that referenced this pull request Sep 30, 2024
@aabluedragon
Copy link

aabluedragon commented Sep 30, 2024

Just stumbled upon it myself, when users choose a nick such as (-_-) it would leave an entry in the nick list of matrix-org-irc which would not clean up until you restart the process.
After merging this PR into my own branch, the problem is gone.

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.

3 participants