Restore away msg, /names sort #237
Conversation
NickServ, we retry joining on a 477 response from the server. Closes Sorted nickname list on /names command lexigraphically. Requires new crate - lexical-sort. Closes osa1#235. Cleaned up some TODOs by adding logs in state.rs.
osa1
left a comment
There was a problem hiding this comment.
I wish we had a way to test this stuff ...
| match parse_servername(params) { | ||
| None => { | ||
| // TODO: Log | ||
| error!("Could not parse server name in 002 RPL_YOURHOST message."); |
There was a problem hiding this comment.
This case is more interesting, and I think I found a potential bug while looking at uses of self.servername. Currently we use the server name when sending pings, and we don't send pings if we're unable to parse the server name from RPL_YOURHOST, but sending pings is pretty important as that's how we check connectivity. I think if we're unable to parse server name we can use the server address as the ping target. Or maybe just send a PING without an argument and wait for an error message from the server (which should reset the ping timer). For example, this works fine to check connectivity:
>>> PING <-- sent message
13:33 cherryh.freenode.net: osa1 PING Not enough parameters
When we receive this error message the pinger timer will be reset.
Only concern is if we can't parse a server name and send PING messages without a target then the server tab will have lots of "PING not enough parameters" lines. Maybe that's fine though as this should be a rare case.
For this PR I think it's fine to add an error-level log here.
|
Here's a screenshot of me trying to join a channel that requires registration but my nick is not registered and
This goes on forever. I guess we'll need to stop trying after a few attempts. It'd also be good to have some delay between attempts. I wonder how sopel handles this? @trevarj also if you split this into two PRs we can merge the rest of the PR. |
|
Removed changes for #236 to merge other changes. |
|
For the record, with the new dependency tiny binary sizes goes from 6372912 bytes to 6865872 bytes. That's +0.5M, +7.7%. |
|
Here's an idea: IRC is an ASCII-based protocol, so nicks are supposed to be ASCII. I just tried using a non-ASCII nick and it's rejected: I think this is the correct behavior. If we assume ASCII strings then we don't need new dependencies, we could simply sort using ASCII codes. Only tricky part is lower and upper case letters, which should be easy enough to handle without requiring 0.5M of dependencies. |
… ascii only nicknames
Ok, great. I reverted and now I'm using regular sort on Strings. |
| error!("Could not find channel index in get_chan_nicks."); | ||
| vec![] | ||
| } | ||
| Some(chan_idx) => { |
There was a problem hiding this comment.
Sorting here now to leave the logic out of the cmd.rs module.
Using unstable because it should be faster and we don't need to worry about equal elements (when it becomes unstable)
|
Thanks! |
Restore away status if reconnected. Closes #234.
If a channel is restricted and we can't autojoin until identified with NickServ, we retry joining on a 477 response from the server. Closes #236Sorted nickname list on /names command lexigraphically. Requires new
crate - lexical-sort. Closes #235.
Cleaned up some TODOs by adding logs in state.rs.