Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

fix undefined nick on pylink when / is in nickname #56

Closed
wants to merge 2 commits into from

Conversation

xnite
Copy link

@xnite xnite commented Mar 9, 2018

Pylink uses /'s in nicknames causing undefined nicknames. This will fix that behavior.

@xnite
Copy link
Author

xnite commented Mar 9, 2018

Not sure why build broke, only added \/ to the nickname regex... :/

@Throne3d
Copy link
Owner

Throne3d commented Mar 9, 2018

I'm confused – you put it in the strict parsing section, which is supposed to conform to the IRC spec, and does not permit forward slashes in nicknames. Could you move it to the unstrict parsing section, i.e. the regex two lines below, which is for nonstandard extensions?

The build broke due to linting, not due to a code error. It thinks the forward slash doesn't need to be escaped. Checking this briefly in Chrome's development console, to see how JavaScript parses the regex, it looks like the forward slash doesn't, in fact, need to be escaped where it is. This seems to be because the slash is enclosed inside the character class – that is, it's in a […] bracketed section – so JavaScript doesn't parse it as a regex-ending slash. Could you remove the backslash escaping it?

I'd appreciate if you could fix this as detailed above, and also add a test to ensure this section of code parses the forward slash.

I'm slightly irked, separately, that so many IRC systems add additional (and, as far as I can tell, non-standard) characters to nicknames, but that's not your fault. (I can understand it in the case of extending it with Unicode support, but I would not expect more basic ASCII characters like slashes to be permitted.) In case anybody's looking for documentation that PyLink does actually do this: https://github.com/GLolol/PyLink/blob/c35c8cd4aa481cbb479c42c92f241b9bedf8e530/docs/faq.md#my-ircd-squits-the-relay-server-with-errors-like-bad-nickname-introduced

Edit: Could you also capitalize the start of your commit message? i.e. "Fix undefined…"

@coveralls
Copy link

coveralls commented Mar 9, 2018

Coverage Status

Coverage remained the same at 94.41% when pulling a0ff6bb on xnite:master into c1825fd on Throne3d:master.

Throne3d added a commit that referenced this pull request Mar 13, 2018
@Throne3d
Copy link
Owner

Merged here: 0780dab

@Throne3d Throne3d closed this Mar 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants