Closed Bug 954905 Opened 10 years ago Closed 10 years ago

Handle a ChanServ entry message

Categories

(Chat Core :: IRC, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

Details

Attachments

(1 file, 3 obsolete files)

*** Original post on bio 1472 at 2012-05-31 01:43:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Blocks: 954155
Attached patch WIP v1 (obsolete) — Splinter Review
*** Original post on bio 1472 as attmnt 1536 at 2012-05-31 01:43:00 UTC ***

#frenchmoz has a notice when you join telling you to use UTF-8 (or something like that, I don't speak French ;)). This currently shows up in a separate tab and is generally annoying (to flo), we should attempt to parse some of the ChanServ/NickServ messages, as bug 954155 (bio 720) suggests.

This includes a quick attempt to parse this specific message.

This patch also fixes a few typos with the DCC handling. (Which is honestly mostly untested, as we don't really do file transfer...)

Not that it'll help, but Anope is: http://www.anope.org/docgen/1.8/
Attachment #8353291 - Flags: feedback?
*** Original post on bio 1472 at 2012-05-31 09:34:32 UTC ***

Thanks for this :-)

Should we detect these messages only when they are NOTICE and only if a conversation for that channel already exists?

To simplify testing without causing noise in a real channel, I registered #testib and put an entry message there.
*** Original post on bio 1472 at 2012-05-31 12:14:12 UTC ***

(In reply to comment #1)
> Thanks for this :-)
> 
> Should we detect these messages only when they are NOTICE and only if a
> conversation for that channel already exists?
Yes, we should probably do that. If the conversation does not exist, should we just eat the message and not display it anywhere (or let it fall through to ircBase and show up in the ChanServ tab)?

> To simplify testing without causing noise in a real channel, I registered
> #testib and put an entry message there.
Good thing I use #testib2 for testing. :) Thanks though, I won't bother #frenchmoz anymore. ;)

A few other things talked about on IRC [1]:
 - Florian was concerned about saying the message is from ChanServ when ChanServ isn't in the conversation (as some of the conversation code might not like this). (The alternative would be to show it as a system message.) I think the right thing to do is show it as a message from ChanServ, and technically IRC messages don't have to come from participants, they can come from outside a channel, so hopefully everything supports this anyway. :)
 - This has only been tested on moznet (which uses Anope for services: this can be confirmed by running /msg NickServ HELP REGISTER, see [2]). It needs to be tested on at least Freenode.
 - I was split on calling the "servicesBase" object as "servicesAnope" object (and expecting to have to make a new services object for each different one). This can always be done at a later date.
 - The abstraction is excessive for just handling a single ChanServ response, but I plan to expand this!
 - I think we should load the NickServ/ChanServ/etc names from preferences, but I'm not sure this mapping is even entirely necessary or if it's overkill. Any opinions? (Do networks ever rename these?)
 - Some statements can be localized. :( [3]

[1] http://log.bezut.info/instantbird/120531/#m150
[2] http://pastebin.instantbird.com/43740
[3] http://anope.git.sourceforge.net/git/gitweb.cgi?p=anope/anope;a=tree;f=lang;h=50474e96eb847551b01972c36b40719814747dd2;hb=HEAD
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Summary: Handle a ChanServ notice → Handle a ChanServ entry message
Attached patch Patch v1 (obsolete) — Splinter Review
*** Original post on bio 1472 as attmnt 1538 at 2012-06-01 01:34:00 UTC ***

Tested on freenode (it works). This also now only works for NOTICEs, not PRIVMSGs. I don't think services EVER use PRIVMSG.

We now also bail out early if the conversation doesn't exist.
Attachment #8353293 - Flags: review?(florian)
Comment on attachment 8353291 [details] [diff] [review]
WIP v1

*** Original change on bio 1472 attmnt 1536 at 2012-06-01 01:34:22 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353291 - Attachment is obsolete: true
Attachment #8353291 - Flags: feedback?
Comment on attachment 8353293 [details] [diff] [review]
Patch v1

*** Original change on bio 1472 attmnt 1538 at 2012-06-01 09:26:18 UTC ***

>diff --git a/chat/protocols/irc/ircServices.jsm b/chat/protocols/irc/ircServices.jsm

>+/*
>+ * This attempts to handle dealing with IRC services, which are a diverse set of
>+ * programs to automate and add features to IRCd. Often these serivces are seen

serivces -> services

>+var servicesBase = {
[...]
>+  commands: {
>+    "ChanServ": function(aMessage) {
>+      // [<channel name>] <message>
>+      let channel = aMessage.params[1].split(" ", 1)[0];
>+      if (channel.length && channel[0] == "[" && channel.slice(-1)[0] == "]") {

This looks like it would want to be the complete test and return false early, to reduce the indent level of the rest of the function.

>+        channel = channel.slice(1, -1);
>+        if (this.isMUCName(channel)) {
>+          // If the conversation doesn't exist...just do nothing.
>+          if (!this.hasConversation(channel))
>+            return true;

I'm a bit concerned by dropping messages silently.
Would it be difficult to make the message go to the server tab?
If it's too difficult (= requires more than 30s of thinking), and this situation is extremely unlikely (it is, if I understood correctly), maybe just return false so that we still open the ChanServ tab in that rare case.

>+  }
>+}

Missing semi-colon here.
Attachment #8353293 - Flags: review?(florian)
Attachment #8353293 - Flags: review-
Attachment #8353293 - Flags: feedback+
*** Original post on bio 1472 at 2012-06-01 09:31:24 UTC ***

(In reply to comment #2)

>  - Some statements can be localized. :( [3]

> [3]
> http://anope.git.sourceforge.net/git/gitweb.cgi?p=anope/anope;a=tree;f=lang;h=50474e96eb847551b01972c36b40719814747dd2;hb=HEAD

I think we can handle only en-US for now, and get most of the benefit, as I would expect most if not all relevant IRC networks to be international, and as a consequence to have their server software set-up in en-US.
*** Original post on bio 1472 at 2012-06-01 09:32:50 UTC ***

Comment on attachment 8353293 [details] [diff] [review] (bio-attmnt 1538)
Patch v1

>diff --git a/chat/protocols/irc/ircServices.jsm b/chat/protocols/irc/ircServices.jsm

>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>+ * You can obtain one at http://mozilla.org/MPL/2.0/. */

This license header isn't an exact copy of the official MPL2 header: http://www.mozilla.org/MPL/headers/

The "file," word is on the third line in the official header.
Attached patch Patch v2 (obsolete) — Splinter Review
*** Original post on bio 1472 as attmnt 1540 at 2012-06-01 10:32:00 UTC ***

Now returns early after each test, fixed typo + header. No longer silently drops messages.
Attachment #8353295 - Flags: review?(florian)
Comment on attachment 8353293 [details] [diff] [review]
Patch v1

*** Original change on bio 1472 attmnt 1538 at 2012-06-01 10:32:35 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353293 - Attachment is obsolete: true
Attached patch Patch v3Splinter Review
*** Original post on bio 1472 as attmnt 1542 at 2012-06-01 10:48:00 UTC ***

I botched the logic on the last patch. :(
Attachment #8353297 - Flags: review?(florian)
Comment on attachment 8353295 [details] [diff] [review]
Patch v2

*** Original change on bio 1472 attmnt 1540 at 2012-06-01 10:48:49 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353295 - Attachment is obsolete: true
Attachment #8353295 - Flags: review?(florian)
Comment on attachment 8353297 [details] [diff] [review]
Patch v3

*** Original change on bio 1472 attmnt 1542 at 2012-06-01 10:50:14 UTC ***

I'm excited to see this is ready for nightlies! :)
Attachment #8353297 - Flags: review?(florian) → review+
*** Original post on bio 1472 at 2012-06-01 23:29:24 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/7e8c665dda1b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.