[Konversation-devel] Review Request: Don't join channels if their name is not prefixed with any of the "channel types"

[Martin Blumenstingl]

> On Oct. 23, 2011, 6:47 p.m., Eike Hein wrote:
> > src/irc/server.cpp, line 3624
> > <http://git.reviewboard.kde.org/r/102951/diff/2/?file=39572#file39572line3624>
> >
> >     Please don't remove this check. There's no need to do all the work generateJoinCommand() does if there are no channels.

Right, might save some µs.
Please note that the second check must be done outside the if (but it's commented why :-)).


> On Oct. 23, 2011, 6:47 p.m., Eike Hein wrote:
> > src/irc/server.h, line 602
> > <http://git.reviewboard.kde.org/r/102951/diff/2/?file=39571#file39571line602>
> >
> >     Just "JOIN commands", also below.


> On Oct. 23, 2011, 6:47 p.m., Eike Hein wrote:
> > src/irc/server.h, line 604
> > <http://git.reviewboard.kde.org/r/102951/diff/2/?file=39571#file39571line604>
> >
> >     "invalid channels as per CHANTYPES" for good measure.

Good point - done.


> On Oct. 23, 2011, 6:47 p.m., Eike Hein wrote:
> > src/irc/server.h, line 609
> > <http://git.reviewboard.kde.org/r/102951/diff/2/?file=39571#file39571line609>
> >
> >     Why move this away from updateAutoJoin()? It's convenient to have related methods be grouped in the header.

I moved it to the protected methods as nothing outside uses them.
But you're right, it might make sense to keep them grouped (and since updateAutoJoin() is accessed from outside I'll also leave it public).


> On Oct. 23, 2011, 6:47 p.m., Eike Hein wrote:
> > src/irc/server.cpp, line 1052
> > <http://git.reviewboard.kde.org/r/102951/diff/2/?file=39572#file39572line1052>
> >
> >     Why remove this check?

Same as below.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102951/#review7564
-----------------------------------------------------------


On Oct. 25, 2011, 9:12 p.m., Martin Blumenstingl wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102951/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2011, 9:12 p.m.)
> 
> 
> Review request for Konversation.
> 
> 
> Description
> -------
> 
> Konversation tried to join all channels given in the "auto join channel"-list.
> The channel name must be prefixed with any of the "channel types" (often "#" is used here).
> 
> Some IRCDs ignore the whole JOIN-command if any of the channels in there is invalid.
> Thus we filter out invalid channels when building the JOIN-command so at least all valid channels are joined.
> 
> 
> Diffs
> -----
> 
>   src/irc/server.h 6f4bb9b 
>   src/irc/server.cpp 353c6e6 
> 
> Diff: http://git.reviewboard.kde.org/r/102951/diff/diff
> 
> 
> Testing
> -------
> 
> Tested if the JOIN-command on freenode still works, even if a channel called "invalid" is in the "auto join channel"-list.
> Worked fine.
> 
> 
> Thanks,
> 
> Martin Blumenstingl
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konversation-devel/attachments/20111025/794b2914/attachment.html>