irc::proto: errors on `MODE #chan` client query

[martinetd]

Hello!

I know #22 has been closed due to lack of interest/help, but the IrcCodec has is mostly valid as is for servers and has been of great help to reuse as a building block for me (toy project: https://github.com/martinetd/matrirc - don't look it's horrible :D)

So far it's mostly working, the only error I get is that irssi inquires about channel modes with MODE #chan without any modifier, and I get an InvalidMessage { string: "MODE #test\r\n", cause: InvalidModeString { string: "", cause: MissingModeModifier } } Right now I don't care about this so just ignore errors but ideally would like to warn about them so handling "known errors" would make sense to me.

As a more valid use-case for this crate though, if irssi uses it, it probably means it's valid and a client could want to query about channel modes for some reason, so we should allow building a MODE commande with an empty modes parameter:

MODE #foo
:irc.whatever.org 324 mynick #foo + 
:irc.whatever.org 329 mynick #foo 1601883174
(first is mode list, second is channel creation timestamp ; interestingly you don't have to be in channel to ask, typical irc... alternatively this:)
:irc.whatever.org 401 mynick #foo :No such nick/channel

Would you consider allowing that for the next breaking version? I'm not in a hurry :P

(Happy to send a PR for what would basically be the following (untested), but figured I'd rather ask first:

diff --git a/irc-proto/src/mode.rs b/irc-proto/src/mode.rs
index a22d5ceb9756..010ceb1ba103 100644
--- a/irc-proto/src/mode.rs
+++ b/irc-proto/src/mode.rs
@@ -287,10 +287,8 @@ where
                 })
             }
             None => {
-                return Err(InvalidModeString {
-                    string: pieces.join(" ").to_owned(),
-                    cause: MissingModeModifier,
-                })
+                // No modifier
+                return Ok(res);
             }
         };

@@ -318,9 +316,7 @@ where

         Ok(res)
     } else {
-        Err(InvalidModeString {
-            string: pieces.join(" ").to_owned(),
-            cause: MissingModeModifier,
-        })
+        // No modifier
+        Ok(res)
     }
 }

)

[aatxe]

Yeah, I think it's fine to take no modifier at all as being an inquiry.

[aatxe]

Just bumping this to say that I would accept such a pull request (given testing). While #22 was abandoned, I would like the protocol crate to be sufficient for anyone wishing to actually build a server, and I agree that the current behavior is actually incorrect even from a client perspective.