Numeric parsing about phergie-irc-parser HOT 6 CLOSED

What would be the benefit of making this change? Can you provide an example use case? As you've pointed out, a drawback would be potential backward compatibility breakage. I'm hesitant to make a change that doesn't result in some positive result for the end user.

Additionally, assuming some action will be taken to resolve this issue, I'd like to get a solution merged prior to a release being tagged to allow for phergie/phergie-irc-client-react#31 to be merged.

from phergie-irc-parser.

It's just logically inconsistent, since the bot's nickname is not a true parameter of the specific numeric event itself, it's just part of the structure for numeric replies. It is never going to be of any use for event handlers that use the parameters.

There would be no harm in extracting it to a separate $parsed element ('target' maybe?), but I don't think it's useful always having it as the first element in $parsed['params'], as it's only going to need to be discarded whenever the params are actually processed by an event handler.

On the breakage front, only one core Phergie plugin actually processes individual params for a numeric event, which is UserMode, as well as one non-core plugin on the wiki, OpsWorksDeployment (@blopboard), which would just need the relevant parameter index decreasing by 1.

from phergie-irc-parser.

'target' would probably make sense. Is this something you'd be willing to submit a PR for?

from phergie-irc-parser.

Sure, will do.

from phergie-irc-parser.

PR looks good. Could you provide specifics on what areas of the OpsWorksDeployment and UserModes plugins would require updates, maybe submit PRs for them as well? In doing a quick skim of both, it seems like they're all using named parameters.

from phergie-irc-parser.

UserMode's parsing of RPL_NAMREPLY in loadUserModes() used array_shift() on the params array, and then ran a foreach on the remainder. This actually already assumed that the first parameter was #channel and not TargetUser, so it was broken before anyway!

However, it was broken for other reasons as well, ie. assuming that the (=|*|@) channel privacy sigil was prepended to #channel rather than its own individual param. I have addressed this as part of a set of fixes and improvements for UserMode [branch] which is dependent on the PROTOCTL changes as well as on this change to the parser. Will need some tags for dependency management, so once those are in place, I will update composer.json and request the merge.

OpsWorksDeployment would need a very simple change to its topicPrompt() method, which listens for ERR_CHANOPRIVSNEEDED:

diff --git a/src/Plugin.php b/src/Plugin.php
index 0584770..50b6feb 100644
--- a/src/Plugin.php
+++ b/src/Plugin.php
@@ -162,7 +162,7 @@ class Plugin extends AbstractPlugin implements LoopAwareInterface
     public function topicPrompt(EventInterface $event, Queue $queue)
     {
         $params = $event->getParams();
-        $channel = $params[1];
+        $channel = $params[0];
         $message = 'Can\'t change topic, please run /MODE ' . $channel . ' -t';
         $queue->ircPrivmsg($channel, $message);
     }

from phergie-irc-parser.