Status

defect
VERIFIED FIXED
18 years ago
2 years ago

People

(Reporter: hwaara, Assigned: bugzilla-mozilla-20000923)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: cz-patch)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

18 years ago
I'd like to have a command to ignore people on IRC. /ignore is the widely used
standard command for this feature, AFAIK.

Updated

18 years ago
OS: Windows 98 → All
Hardware: PC → All

Comment 1

17 years ago
What's going on with this?

Comment 2

17 years ago
Subscribe to the request for /ignore.
(spammers on IRC are very annoying and very persistent)
Alias: ignore

Comment 3

16 years ago
This kind of command is necessary, but it seems there's no standard.

/ignore [on|off] nickname

Is this enough?

Comment 4

16 years ago
That'd probably be sufficient. Some clients provide more features (for example,
ignoring only PMs, only channel text, only CTCPs, etc.) but that sort of thing
could be added later.

Comment 5

16 years ago
I'd prefer /ignore <nickname> on|off, so that we have the ability to take more
parameters later.

Comment 6

16 years ago
Posted patch patch v1 (obsolete) — Splinter Review
(1) I can't write help message, because I'm not a native English speaker.
I need help.

(2) I'd found "ignore <nick> <on|off>" isn't consistent with stalk/unstalk.
Should it be ignore/unignore?
(Assignee)

Comment 7

16 years ago
I'm going to have a go at adapting Koike Kazuhiko's patch for the new command
manager.
(Assignee)

Comment 8

16 years ago
--> me, patch soon. :)
Assignee: rginda → silver
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 9

16 years ago
This patch adds two commands, /ignore [<mask>] and /unignore <mask>. These
manipulate the ignoreList on a network, and support full hostmasks as well as
simple nicknames.

When a message comes in, if it has a user as a source, it is checked against a
cache of hostmasks, and if it's not in the list, it's checked against the
actual ignore list. This means each source hostmask is only checked once (until
the ignore list is changed), which improves the speed since the checking of the
hostmask is quite complicated.

Messages of type PRIVMSG and NOTICE are dropped here, but all others are let
throught, but if needed, can be ignored if e.ignored is true (e.g. for mode
changes it could just not show the message, but still keep track of the actual
change internally).
(Assignee)

Updated

16 years ago
Whiteboard: cz-patch

Comment 10

16 years ago
Looks great, a few comments:

Why the big try/catch in onRawData?  What's going to raise an error?  Maybe the
try/catch can be made smaller.  With a block that size, it's easy to make a
mistake when changing the routine, and not notice a new, unexpected exception.

makeRegExpSafe... didn't we do something like that for the stalk list, too? 
This could probably move into utils.js, and return a RegExp object instead of a
converted string.  I'd suggest that "?" should translate to ".", and "*" should
become ".*", so that the effect matches what users would expect from a blob search.

As a general rule, I usually put nested functions at the top of the the
enclosing function, before any code in the function, as opposed to defining them
in a block buried later on in the function.  Also, ending nested functions with
a ";" aftert he final "}" makes emacs happier wrt magic indenting.

I'd lean twords seeing the "if (e.ignored) return" type logic move into
handlers.js, so that we could do the actual ignoring with css at some point. 
The would let users retroactively ignore/unignore.  The downside is that we
might need more if (e.ignored) checks.  Like, should we log an ignored message?
 Probably not, I guess.

I don't like how commands.js needs to clear the cache either (I see your FIXME
comment in there.)  How about irc networks (servers?, nah, probably networks)
get a .ignore() and a .unignore method instead, and let all the smarts live
there?  It would be nice to cache the mask RegExp objects on the network too, so
we don't have to build three regexps for each message that comes in.
(Assignee)

Comment 11

16 years ago
Posted patch Patch v3.Splinter Review
This patch is an update of attachement 131263, and should basically cover all
the points in comment 10.

There are a couple of other changes, such as putting ^ and $ into the RegExps,
and using the case-insensitive option. It also does not keep RegExp objects for
items in the mask that are just "*", it stores null instead - this reduces
memory usage, and saves a call or two to .match in the matching code.

I have left the ignoring of PRIVMSG and NOTICE messages where they were,
because I feel ignoring should as early as possible. Other messages could be
ignored later, like NICK for example.

Note: this diff is made from CVS, it won't apply to the latest version from
hacksrus.com, due to diff being rather bad at picking lines. It should only be
the cmdIgnore hunk that has problems, though, and can be patched manually.
(Assignee)

Updated

16 years ago
Attachment #126174 - Attachment is obsolete: true
Attachment #131263 - Attachment is obsolete: true

Comment 12

16 years ago
Regarding doing the ignore with CSS.
Wouldn't that still result in wasted buffer space and log space?
If you're ignoring someone for flooding, or just being too talkative, that'd be
inconvenient.

Comment 13

16 years ago
Comment on attachment 136500 [details] [diff] [review]
Patch v3.

yeah, that looks about right ;)  r=rginda
Attachment #136500 - Flags: review+
(Assignee)

Comment 14

16 years ago
Comment on attachment 136500 [details] [diff] [review]
Patch v3.

Checked in, and in XPI 0.9.58 on website.
(Assignee)

Updated

16 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Product: Core → Other Applications

Updated

14 years ago
Status: RESOLVED → VERIFIED
Comment hidden (obsolete)
You need to log in before you can comment on or make changes to this bug.