Re-work Handlers for IRC events; add Commands.

client/connection.go

@@ -5,7 +5,6 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
-	"github.com/fluffle/goevent/event"
 	"github.com/fluffle/goirc/state"
 	"github.com/fluffle/golog/logging"
 	"net"
@@ -20,16 +19,14 @@ type Conn struct {
 	Me      *state.Nick
 	Network string
 
-	// Replaceable function to customise the 433 handler's new nick
-	NewNick func(string) string
-
-	// Event handler registry and dispatcher
-	ER event.EventRegistry
-	ED event.EventDispatcher
+	// Handlers and Commands
+	handlers *hSet
+	commands *cSet
 
 	// State tracker for nicks and channels
-	ST state.StateTracker
-	st bool
+	ST         state.StateTracker
+	st         bool
+	stRemovers []Remover
 
 	// Use the State field to store external state that handlers might need.
 	// Remember ... you might need locking for this ;-)
@@ -50,9 +47,15 @@ type Conn struct {
 	SSL       bool
 	SSLConfig *tls.Config
 
+	// Replaceable function to customise the 433 handler's new nick
+	NewNick func(string) string
+
 	// Client->server ping frequency, in seconds. Defaults to 3m.
 	PingFreq time.Duration
 
+	// Controls what is stripped from line.Args[1] for Commands
+	CommandStripNick, CommandStripPrefix bool
+
 	// Set this to true to disable flood protection and false to re-enable
 	Flood bool
 
@@ -62,9 +65,9 @@ type Conn struct {
 }
 
 // Creates a new IRC connection object, but doesn't connect to anything so
-// that you can add event handlers to it. See AddHandler() for details.
-func SimpleClient(nick string, args ...string) *Conn {
-	r := event.NewRegistry()
+// that you can add event handlers to it. See AddHandler() for details
+func Client(nick string, args ...string) *Conn {
+	logging.InitFromFlags()
 	ident := "goirc"
 	name := "Powered by GoIRC"
 
@@ -74,30 +77,18 @@ func SimpleClient(nick string, args ...string) *Conn {
 	if len(args) > 1 && args[1] != "" {
 		name = args[1]
 	}
-	return Client(nick, ident, name, r)
-}
-
-func Client(nick, ident, name string, r event.EventRegistry) *Conn {
-	if r == nil {
-		return nil
-	}
-	logging.InitFromFlags()
 	conn := &Conn{
-		ER:        r,
-		ED:        r,
-		st:        false,
-		in:        make(chan *Line, 32),
-		out:       make(chan string, 32),
-		cSend:     make(chan bool),
-		cLoop:     make(chan bool),
-		cPing:     make(chan bool),
-		SSL:       false,
-		SSLConfig: nil,
-		PingFreq:  3 * time.Minute,
-		Flood:     false,
-		NewNick:   func(s string) string { return s + "_" },
-		badness:   0,
-		lastsent:  time.Now(),
+		in:         make(chan *Line, 32),
+		out:        make(chan string, 32),
+		cSend:      make(chan bool),
+		cLoop:      make(chan bool),
+		cPing:      make(chan bool),
+		handlers:   handlerSet(),
+		commands:   commandSet(),
+		stRemovers: make([]Remover, 0, len(stHandlers)),
+		PingFreq:   3 * time.Minute,
+		NewNick:    func(s string) string { return s + "_" },
+		lastsent:   time.Now(),
 	}
 	conn.addIntHandlers()
 	conn.Me = state.NewNick(nick)
@@ -257,7 +248,7 @@ func (conn *Conn) runLoop() {
 	for {
 		select {
 		case line := <-conn.in:
-			conn.ED.Dispatch(line.Cmd, conn, line)
+			conn.dispatch(line)
 		case <-conn.cLoop:
 			// strobe on control channel, bail out
 			return
@@ -314,7 +305,7 @@ func (conn *Conn) shutdown() {
 	// as calling sock.Close() will cause recv() to recieve EOF in readstring()
 	if conn.Connected {
 		logging.Info("irc.shutdown(): Disconnected from server.")
-		conn.ED.Dispatch("disconnected", conn, &Line{})
+		conn.dispatch(&Line{Cmd: "disconnected"})
 		conn.Connected = false
 		conn.sock.Close()
 		conn.cSend <- true

client/connection_test.go

@@ -3,7 +3,6 @@ package client
 import (
 	"bufio"
 	"code.google.com/p/gomock/gomock"
-	"github.com/fluffle/goevent/event"
 	"github.com/fluffle/golog/logging"
 	"github.com/fluffle/goirc/state"
 	"strings"
@@ -14,7 +13,6 @@ import (
 type testState struct {
 	ctrl *gomock.Controller
 	st   *state.MockStateTracker
-	ed   *event.MockEventDispatcher
 	nc   *mockNetConn
 	c    *Conn
 }
@@ -22,13 +20,10 @@ type testState struct {
 func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
 	ctrl := gomock.NewController(t)
 	st := state.NewMockStateTracker(ctrl)
-	r := event.NewRegistry()
-	ed := event.NewMockEventDispatcher(ctrl)
 	nc := MockNetConn(t)
-	c := Client("test", "test", "Testing IRC", r)
+	c := Client("test", "test", "Testing IRC")
 	logging.SetLogLevel(logging.LogFatal)
 
-	c.ED = ed
 	c.ST = st
 	c.st = true
 	c.sock = nc
@@ -42,15 +37,14 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
 		<-time.After(1e6)
 	}
 
-	return c, &testState{ctrl, st, ed, nc, c}
+	return c, &testState{ctrl, st, nc, c}
 }
 
 func (s *testState) tearDown() {
-	s.ed.EXPECT().Dispatch("disconnected", s.c, &Line{})
 	s.st.EXPECT().Wipe()
 	s.nc.ExpectNothing()
 	s.c.shutdown()
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
 	s.ctrl.Finish()
 }
 
@@ -61,56 +55,61 @@ func TestEOF(t *testing.T) {
 	// Since we're not using tearDown() here, manually call Finish()
 	defer s.ctrl.Finish()
 
+	// Set up a handler to detect whether disconnected handlers are called
+	dcon := false
+	c.HandleFunc("disconnected", func (conn *Conn, line *Line) {
+		dcon = true
+	})
+
 	// Simulate EOF from server
-	s.ed.EXPECT().Dispatch("disconnected", c, &Line{})
 	s.st.EXPECT().Wipe()
 	s.nc.Close()
 
 	// Since things happen in different internal goroutines, we need to wait
 	// 1 ms should be enough :-)
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
 
 	// Verify that the connection no longer thinks it's connected
 	if c.Connected {
 		t.Errorf("Conn still thinks it's connected to the server.")
 	}
+
+	// Verify that disconnected handler was called
+	if !dcon {
+		t.Errorf("Conn did not call disconnected handlers.")
+	}
 }
 
 func TestClientAndStateTracking(t *testing.T) {
-	// This doesn't use setUp() as we want to pass in a mock EventRegistry.
 	ctrl := gomock.NewController(t)
-	r := event.NewMockEventRegistry(ctrl)
 	st := state.NewMockStateTracker(ctrl)
-
-	for n, _ := range intHandlers {
-		// We can't use EXPECT() here as comparisons of functions are
-		// no longer valid in Go, which causes reflect.DeepEqual to bail.
-		// Instead, ignore the function arg and just ensure that all the
-		// handler names are correctly passed to AddHandler.
-		ctrl.RecordCall(r, "AddHandler", gomock.Any(), n)
-	}
-	c := Client("test", "test", "Testing IRC", r)
+	c := Client("test", "test", "Testing IRC")
 
 	// Assert some basic things about the initial state of the Conn struct
-	if c.ER != r || c.ED != r || c.st != false || c.ST != nil {
-		t.Errorf("Conn not correctly initialised with external deps.")
-	}
-	if c.in == nil || c.out == nil || c.cSend == nil || c.cLoop == nil {
-		t.Errorf("Conn control channels not correctly initialised.")
-	}
 	if c.Me.Nick != "test" || c.Me.Ident != "test" ||
 		c.Me.Name != "Testing IRC" || c.Me.Host != "" {
 		t.Errorf("Conn.Me not correctly initialised.")
 	}
-
-	// OK, while we're here with a mock event registry...
-	for n, _ := range stHandlers {
-		// See above.
-		ctrl.RecordCall(r, "AddHandler", gomock.Any(), n)
+	// Check that the internal handlers are correctly set up
+	for k, _ := range intHandlers {
+		if _, ok := c.handlers.set[strings.ToLower(k)]; !ok {
+			t.Errorf("Missing internal handler for '%s'.", k)
+		}
 	}
-	c.EnableStateTracking()
 
-	// We're expecting the untracked me to be replaced by a tracked one.
+	// Now enable the state tracking code and check its handlers
+	c.EnableStateTracking()
+	for k, _ := range stHandlers {
+		if _, ok := c.handlers.set[strings.ToLower(k)]; !ok {
+			t.Errorf("Missing state handler for '%s'.", k)
+		}
+	}
+	if len(c.stRemovers) != len(stHandlers) {
+		t.Errorf("Incorrect number of Removers (%d != %d) when adding state handlers.",
+			len(c.stRemovers), len(stHandlers))
+	}
+
+	// We're expecting the untracked me to be replaced by a tracked one
 	if c.Me.Nick != "test" || c.Me.Ident != "test" ||
 		c.Me.Name != "Testing IRC" || c.Me.Host != "" {
 		t.Errorf("Enabling state tracking did not replace Me correctly.")
@@ -119,18 +118,25 @@ func TestClientAndStateTracking(t *testing.T) {
 		t.Errorf("State tracker not enabled correctly.")
 	}
 
-	// Now, shim in the mock state tracker and test disabling state tracking.
+	// Now, shim in the mock state tracker and test disabling state tracking
 	me := c.Me
 	c.ST = st
 	st.EXPECT().Wipe()
-	for n, _ := range stHandlers {
-		// See above.
-		ctrl.RecordCall(r, "DelHandler", gomock.Any(), n)
-	}
 	c.DisableStateTracking()
 	if c.st || c.ST != nil || c.Me != me {
 		t.Errorf("State tracker not disabled correctly.")
 	}
+
+	// Finally, check state tracking handlers were all removed correctly
+	for k, _ := range stHandlers {
+		if _, ok := c.handlers.set[strings.ToLower(k)]; ok && k != "NICK" {
+			// A bit leaky, because intHandlers adds a NICK handler.
+			t.Errorf("State handler for '%s' not removed correctly.", k)
+		}
+	}
+	if len(c.stRemovers) != 0 {
+		t.Errorf("stRemovers not zeroed correctly when removing state handlers.")
+	}
 	ctrl.Finish()
 }
 
@@ -202,7 +208,7 @@ func TestRecv(t *testing.T) {
 	// reader is a helper to do a "non-blocking" read of c.in
 	reader := func() *Line {
 		select {
-		case <-time.After(1e6):
+		case <-time.After(time.Millisecond):
 		case l := <-c.in:
 			return l
 		}
@@ -221,7 +227,7 @@ func TestRecv(t *testing.T) {
 
 	// Strangely, recv() needs some time to start up, but *only* when this test
 	// is run standalone with: client/_test/_testmain --test.run TestRecv
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
 
 	// Now, this should mean that we'll receive our parsed line on c.in
 	if l := reader(); l == nil || l.Cmd != "001" {
@@ -245,7 +251,6 @@ func TestRecv(t *testing.T) {
 	if exited {
 		t.Errorf("Exited before socket close.")
 	}
-	s.ed.EXPECT().Dispatch("disconnected", c, &Line{})
 	s.st.EXPECT().Wipe()
 	s.nc.Close()
 
@@ -255,7 +260,7 @@ func TestRecv(t *testing.T) {
 	<-c.cLoop
 	<-c.cPing
 	// Give things time to shake themselves out...
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
 	if !exited {
 		t.Errorf("Didn't exit on socket close.")
 	}
@@ -296,7 +301,7 @@ func TestPing(t *testing.T) {
 		exited = true
 	}()
 
-	// The first ping should be after a second,
+	// The first ping should be after 50ms,
 	// so we don't expect anything now on c.in
 	if s := reader(); s != "" {
 		t.Errorf("Line output directly after ping started.")
@@ -349,15 +354,25 @@ func TestRunLoop(t *testing.T) {
 		bufio.NewReader(c.sock),
 		bufio.NewWriter(c.sock))
 
-	// NOTE: here we assert that no Dispatch event has been called yet by
-	// calling s.ctrl.Finish(). There doesn't appear to be any harm in this.
+	// Set up a handler to detect whether 001 handler is called
+	h001 := false
+	c.HandleFunc("001", func (conn *Conn, line *Line) {
+		h001 = true
+	})
+	// Set up a handler to detect whether 002 handler is called
+	h002 := false
+	c.HandleFunc("002", func (conn *Conn, line *Line) {
+		h002 = true
+	})
+
 	l1 := parseLine(":irc.server.org 001 test :First test line.")
 	c.in <- l1
-	s.ctrl.Finish()
+	if h001 {
+		t.Errorf("001 handler called before runLoop started.")
+	}
 
 	// We want to test that the a goroutine calling runLoop will exit correctly.
 	// Now, we can expect the call to Dispatch to take place as runLoop starts.
-	s.ed.EXPECT().Dispatch("001", c, l1)
 	exited := false
 	go func() {
 		c.runLoop()
@@ -365,15 +380,20 @@ func TestRunLoop(t *testing.T) {
 	}()
 	// Here, the opposite seemed to take place, with TestRunLoop failing when
 	// run as part of the suite but passing when run on it's own.
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
+	if !h001 {
+		t.Errorf("001 handler not called after runLoop started.")
+	}
 
 	// Send another line, just to be sure :-)
 	l2 := parseLine(":irc.server.org 002 test :Second test line.")
-	s.ed.EXPECT().Dispatch("002", c, l2)
 	c.in <- l2
 	// It appears some sleeping is needed after all of these to ensure channel
 	// sends occur before the close signal is sent below...
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
+	if !h002 {
+		t.Errorf("002 handler not called while runLoop started.")
+	}
 
 	// Now, use the control channel to exit send and kill the goroutine.
 	if exited {
@@ -381,13 +401,17 @@ func TestRunLoop(t *testing.T) {
 	}
 	c.cLoop <- true
 	// Allow propagation time...
-	<-time.After(1e6)
+	<-time.After(time.Millisecond)
 	if !exited {
 		t.Errorf("Didn't exit after signal.")
 	}
 
 	// Sending more on c.in shouldn't dispatch any further events
+	h001 = false
 	c.in <- l1
+	if h001 {
+		t.Errorf("001 handler called after runLoop ended.")
+	}
 }
 
 func TestWrite(t *testing.T) {
@@ -432,8 +456,6 @@ func TestWrite(t *testing.T) {
 		<-c.cPing
 	}()
 	s.nc.Close()
-
-	s.ed.EXPECT().Dispatch("disconnected", c, &Line{})
 	s.st.EXPECT().Wipe()
 	c.write("she can't pass unit tests")
 }
@@ -468,13 +490,14 @@ func TestRateLimit(t *testing.T) {
 	// characters as the line length means we should be increasing badness by
 	// 2.5 seconds minus the delta between the two ratelimit calls. This should
 	// be minimal but it's guaranteed that it won't be zero. Use 10us as a fuzz.
-	if l := c.rateLimit(60); l != 0 || abs(c.badness - 25*1e8) > 10 * time.Microsecond {
+	if l := c.rateLimit(60); l != 0 ||
+		abs(c.badness - 2500*time.Millisecond) > 10 * time.Microsecond {
 		t.Errorf("Rate limit calculating badness incorrectly.")
 	}
 	// At this point, we can tip over the badness scale, with a bit of help.
 	// 720 chars => +8 seconds of badness => 10.5 seconds => ratelimit
 	if l := c.rateLimit(720); l != 8 * time.Second ||
-		abs(c.badness - 105*1e8) > 10 * time.Microsecond {
+		abs(c.badness - 10500*time.Millisecond) > 10 * time.Microsecond {
 		t.Errorf("Rate limit failed to return correct limiting values.")
 		t.Errorf("l=%d, badness=%d", l, c.badness)
 	}

client/dispatch.go

@@ -0,0 +1,228 @@
+package client
+
+import (
+	"github.com/fluffle/golog/logging"
+	"strings"
+	"sync"
+)
+
+// An IRC handler looks like this:
+type Handler interface {
+	Handle(*Conn, *Line)
+}
+
+// And when they've been added to the client they are removable.
+type Remover interface {
+	Remove()
+}
+
+type HandlerFunc func(*Conn, *Line)
+
+func (hf HandlerFunc) Handle(conn *Conn, line *Line) {
+	hf(conn, line)
+}
+
+type hList struct {
+	start, end *hNode
+}
+
+type hNode struct {
+	next, prev *hNode
+	set        *hSet
+	event      string
+	handler    Handler
+}
+
+func (hn *hNode) Handle(conn *Conn, line *Line) {
+	hn.handler.Handle(conn, line)
+}
+
+func (hn *hNode) Remove() {
+	hn.set.remove(hn)
+}
+
+type hSet struct {
+	set map[string]*hList
+	sync.RWMutex
+}
+
+func handlerSet() *hSet {
+	return &hSet{set: make(map[string]*hList)}
+}
+
+func (hs *hSet) add(ev string, h Handler) Remover {
+	hs.Lock()
+	defer hs.Unlock()
+	ev = strings.ToLower(ev)
+	l, ok := hs.set[ev]
+	if !ok {
+		l = &hList{}
+	}
+	hn := &hNode{
+		set:     hs,
+		event:   ev,
+		handler: h,
+	}
+	if !ok {
+		l.start = hn
+	} else {
+		hn.prev = l.end
+		l.end.next = hn
+	}
+	l.end = hn
+	hs.set[ev] = l
+	return hn
+}
+
+func (hs *hSet) remove(hn *hNode) {
+	hs.Lock()
+	defer hs.Unlock()
+	l, ok := hs.set[hn.event]
+	if !ok {
+		logging.Error("Removing node for unknown event '%s'", hn.event)
+		return
+	}
+	if hn.next == nil {
+		l.end = hn.prev
+	} else {
+		hn.next.prev = hn.prev
+	}
+	if hn.prev == nil {
+		l.start = hn.next
+	} else {
+		hn.prev.next = hn.next
+	}
+	hn.next = nil
+	hn.prev = nil
+	hn.set = nil
+	if l.start == nil || l.end == nil {
+		delete(hs.set, hn.event)
+	}
+}
+
+func (hs *hSet) dispatch(conn *Conn, line *Line) {
+	hs.RLock()
+	defer hs.RUnlock()
+	ev := strings.ToLower(line.Cmd)
+	list, ok := hs.set[ev]
+	if !ok { return }
+	for hn := list.start; hn != nil; hn = hn.next {
+		go hn.Handle(conn, line)
+	}
+}
+
+// An IRC command looks like this:
+type Command interface {
+	Execute(*Conn, *Line)
+	Help() string
+}
+
+type command struct {
+	fn   HandlerFunc
+	help string
+}
+
+func (c *command) Execute(conn *Conn, line *Line) {
+	c.fn(conn, line)
+}
+
+func (c *command) Help() string {
+	return c.help
+}
+
+type cNode struct {
+	cmd    Command
+	set    *cSet
+	prefix string
+}
+
+func (cn *cNode) Execute(conn *Conn, line *Line) {
+	cn.cmd.Execute(conn, line)
+}
+
+func (cn *cNode) Help() string {
+	return cn.cmd.Help()
+}
+
+func (cn *cNode) Remove() {
+	cn.set.remove(cn)
+}
+
+type cSet struct {
+	set map[string]*cNode
+	sync.RWMutex
+}
+
+func commandSet() *cSet {
+	return &cSet{set: make(map[string]*cNode)}
+}
+
+func (cs *cSet) add(pf string, c Command) Remover {
+	cs.Lock()
+	defer cs.Unlock()
+	pf = strings.ToLower(pf)
+	if _, ok := cs.set[pf]; ok {
+		logging.Error("Command prefix '%s' already registered.", pf)
+		return nil
+	}
+	cn := &cNode{
+		cmd:    c,
+		set:    cs,
+		prefix: pf,
+	}
+	cs.set[pf] = cn
+	return cn
+}
+
+func (cs *cSet) remove(cn *cNode) {
+	cs.Lock()
+	defer cs.Unlock()
+	delete(cs.set, cn.prefix)
+	cn.set = nil
+}
+
+func (cs *cSet) match(txt string) (final Command, prefixlen int) {
+	cs.RLock()
+	defer cs.RUnlock()
+	txt = strings.ToLower(txt)
+	for prefix, cmd := range cs.set {
+		if !strings.HasPrefix(txt, prefix) {
+			continue
+		}
+		if final == nil || len(prefix) > prefixlen {
+			prefixlen = len(prefix)
+			final = cmd
+		}
+	}
+	return
+}
+
+// Handlers are triggered on incoming Lines from the server, with the handler
+// "name" being equivalent to Line.Cmd. Read the RFCs for details on what
+// replies could come from the server. They'll generally be things like
+// "PRIVMSG", "JOIN", etc. but all the numeric replies are left as ascii
+// strings of digits like "332" (mainly because I really didn't feel like
+// putting massive constant tables in).
+func (conn *Conn) Handle(name string, h Handler) Remover {
+	return conn.handlers.add(name, h)
+}
+
+func (conn *Conn) HandleFunc(name string, hf HandlerFunc) Remover {
+	return conn.Handle(name, hf)
+}
+
+func (conn *Conn) Command(prefix string, c Command) Remover {
+	return conn.commands.add(prefix, c)
+}
+
+func (conn *Conn) CommandFunc(prefix string, hf HandlerFunc, help string) Remover {
+	return conn.Command(prefix, &command{hf, help})
+}
+
+func (conn *Conn) dispatch(line *Line) {
+	conn.handlers.dispatch(conn, line)
+}
+
+func (conn *Conn) cmdMatch(txt string) (Command, int) {
+	return conn.commands.match(txt)
+}

client/dispatch_test.go

@@ -0,0 +1,229 @@
+package client
+
+import (
+	"testing"
+	"time"
+)
+
+func TestHandlerSet(t *testing.T) {
+	hs := handlerSet()
+	if len(hs.set) != 0 {
+		t.Errorf("New set contains things!")
+	}
+
+	callcount := 0
+	f := func(c *Conn, l *Line) {
+		callcount++
+	}
+
+	// Add one
+	hn1 := hs.add("ONE", HandlerFunc(f)).(*hNode)
+	hl, ok := hs.set["one"]
+	if len(hs.set) != 1 || !ok {
+		t.Errorf("Set doesn't contain 'one' list after add().")
+	}
+	if hn1.set != hs || hn1.event != "one" || hn1.prev != nil || hn1.next != nil {
+		t.Errorf("First node for 'one' not created correctly")
+	}
+	if hl.start != hn1 || hl.end != hn1 {
+		t.Errorf("Node not added to empty 'one' list correctly.")
+	}
+
+	// Add another one...
+	hn2 := hs.add("one", HandlerFunc(f)).(*hNode)
+	if len(hs.set) != 1 {
+		t.Errorf("Set contains more than 'one' list after add().")
+	}
+	if hn2.set != hs || hn2.event != "one" {
+		t.Errorf("Second node for 'one' not created correctly")
+	}
+	if hn1.prev != nil || hn1.next != hn2 || hn2.prev != hn1 || hn2.next != nil {
+		t.Errorf("Nodes for 'one' not linked correctly.")
+	}
+	if hl.start != hn1 || hl.end != hn2 {
+		t.Errorf("Node not appended to 'one' list correctly.")
+	}
+
+	// Add a third one!
+	hn3 := hs.add("one", HandlerFunc(f)).(*hNode)
+	if len(hs.set) != 1 {
+		t.Errorf("Set contains more than 'one' list after add().")
+	}
+	if hn3.set != hs || hn3.event != "one" {
+		t.Errorf("Third node for 'one' not created correctly")
+	}
+	if hn1.prev != nil || hn1.next != hn2 ||
+		hn2.prev != hn1 || hn2.next != hn3 ||
+		hn3.prev != hn2 || hn3.next != nil {
+		t.Errorf("Nodes for 'one' not linked correctly.")
+	}
+	if hl.start != hn1 || hl.end != hn3 {
+		t.Errorf("Node not appended to 'one' list correctly.")
+	}
+
+	// And finally a fourth one!
+	hn4 := hs.add("one", HandlerFunc(f)).(*hNode)
+	if len(hs.set) != 1 {
+		t.Errorf("Set contains more than 'one' list after add().")
+	}
+	if hn4.set != hs || hn4.event != "one" {
+		t.Errorf("Fourth node for 'one' not created correctly.")
+	}
+	if hn1.prev != nil || hn1.next != hn2 ||
+		hn2.prev != hn1 || hn2.next != hn3 ||
+		hn3.prev != hn2 || hn3.next != hn4 ||
+		hn4.prev != hn3 || hn4.next != nil {
+		t.Errorf("Nodes for 'one' not linked correctly.")
+	}
+	if hl.start != hn1 || hl.end != hn4 {
+		t.Errorf("Node not appended to 'one' list correctly.")
+	}
+
+	// Dispatch should result in 4 additions.
+	if callcount != 0 {
+		t.Errorf("Something incremented call count before we were expecting it.")
+	}
+	hs.dispatch(nil, &Line{Cmd:"One"})
+	<-time.After(time.Millisecond)
+	if callcount != 4 {
+		t.Errorf("Our handler wasn't called four times :-(")
+	}
+
+	// Remove node 3.
+	hn3.Remove()
+	if len(hs.set) != 1 {
+		t.Errorf("Set list count changed after remove().")
+	}
+	if hn3.set != nil || hn3.prev != nil || hn3.next != nil {
+		t.Errorf("Third node for 'one' not removed correctly.")
+	}
+	if hn1.prev != nil || hn1.next != hn2 ||
+		hn2.prev != hn1 || hn2.next != hn4 ||
+		hn4.prev != hn2 || hn4.next != nil {
+		t.Errorf("Third node for 'one' not unlinked correctly.")
+	}
+	if hl.start != hn1 || hl.end != hn4 {
+		t.Errorf("Third node for 'one' changed list pointers.")
+	}
+
+	// Dispatch should result in 3 additions.
+	hs.dispatch(nil, &Line{Cmd:"One"})
+	<-time.After(time.Millisecond)
+	if callcount != 7 {
+		t.Errorf("Our handler wasn't called three times :-(")
+	}
+
+	// Remove node 1.
+	hs.remove(hn1)
+	if len(hs.set) != 1 {
+		t.Errorf("Set list count changed after remove().")
+	}
+	if hn1.set != nil || hn1.prev != nil || hn1.next != nil {
+		t.Errorf("First node for 'one' not removed correctly.")
+	}
+	if hn2.prev != nil || hn2.next != hn4 || hn4.prev != hn2 || hn4.next != nil {
+		t.Errorf("First node for 'one' not unlinked correctly.")
+	}
+	if hl.start != hn2 || hl.end != hn4 {
+		t.Errorf("First node for 'one' didn't change list pointers.")
+	}
+
+	// Dispatch should result in 2 additions.
+	hs.dispatch(nil, &Line{Cmd:"One"})
+	<-time.After(time.Millisecond)
+	if callcount != 9 {
+		t.Errorf("Our handler wasn't called two times :-(")
+	}
+
+	// Remove node 4.
+	hn4.Remove()
+	if len(hs.set) != 1 {
+		t.Errorf("Set list count changed after remove().")
+	}
+	if hn4.set != nil || hn4.prev != nil || hn4.next != nil {
+		t.Errorf("Fourth node for 'one' not removed correctly.")
+	}
+	if hn2.prev != nil || hn2.next != nil {
+		t.Errorf("Fourth node for 'one' not unlinked correctly.")
+	}
+	if hl.start != hn2 || hl.end != hn2 {
+		t.Errorf("Fourth node for 'one' didn't change list pointers.")
+	}
+
+	// Dispatch should result in 1 addition.
+	hs.dispatch(nil, &Line{Cmd:"One"})
+	<-time.After(time.Millisecond)
+	if callcount != 10 {
+		t.Errorf("Our handler wasn't called once :-(")
+	}
+
+	// Remove node 2.
+	hs.remove(hn2)
+	if len(hs.set) != 0 {
+		t.Errorf("Removing last node in 'one' didn't remove list.")
+	}
+	if hn2.set != nil || hn2.prev != nil || hn2.next != nil {
+		t.Errorf("Second node for 'one' not removed correctly.")
+	}
+	if hl.start != nil || hl.end != nil {
+		t.Errorf("Second node for 'one' didn't change list pointers.")
+	}
+
+	// Dispatch should result in NO additions.
+	hs.dispatch(nil, &Line{Cmd:"One"})
+	<-time.After(time.Millisecond)
+	if callcount != 10 {
+		t.Errorf("Our handler was called?")
+	}
+}
+
+func TestCommandSet(t *testing.T) {
+	cs := commandSet()
+	if len(cs.set) != 0 {
+		t.Errorf("New set contains things!")
+	}
+
+	c := &command{
+		fn: func(c *Conn, l *Line) {},
+		help: "wtf?",
+	}
+
+	cn1 := cs.add("ONE", c).(*cNode)
+	if _, ok := cs.set["one"]; !ok || cn1.set != cs || cn1.prefix != "one" {
+		t.Errorf("Command 'one' not added to set correctly.")
+	}
+
+	if fail := cs.add("one", c); fail != nil {
+		t.Errorf("Adding a second 'one' command did not fail as expected.")
+	}
+	
+	cn2 := cs.add("One Two", c).(*cNode)
+	if _, ok := cs.set["one two"]; !ok || cn2.set != cs || cn2.prefix != "one two" {
+		t.Errorf("Command 'one two' not added to set correctly.")
+	}
+
+	if c, l := cs.match("foo"); c != nil || l != 0 {
+		t.Errorf("Matched 'foo' when we shouldn't.")
+	}
+	if c, l := cs.match("one"); c.(*cNode) != cn1 || l != 3 {
+		t.Errorf("Didn't match 'one' when we should have.")
+	}
+	if c, l := cs.match ("one two three"); c.(*cNode) != cn2 || l != 7 {
+		t.Errorf("Didn't match 'one two' when we should have.")
+	}
+
+	cs.remove(cn2)
+	if _, ok := cs.set["one two"]; ok || cn2.set != nil {
+		t.Errorf("Command 'one two' not removed correctly.")
+	}
+	if c, l := cs.match ("one two three"); c.(*cNode) != cn1 || l != 3 {
+		t.Errorf("Didn't match 'one' when we should have.")
+	}
+	cn1.Remove()
+	if _, ok := cs.set["one"]; ok || cn1.set != nil {
+		t.Errorf("Command 'one' not removed correctly.")
+	}
+	if c, l := cs.match ("one two three"); c != nil || l != 0 {
+		t.Errorf("Matched 'one' when we shouldn't have.")
+	}
+}

client/handlers.go

@@ -4,48 +4,23 @@ package client
 // to manage tracking an irc connection etc.
 
 import (
-	"github.com/fluffle/goevent/event"
 	"strings"
 )
 
-// An IRC handler looks like this:
-type IRCHandler func(*Conn, *Line)
-
-// AddHandler() adds an event handler for a specific IRC command.
-//
-// Handlers are triggered on incoming Lines from the server, with the handler
-// "name" being equivalent to Line.Cmd. Read the RFCs for details on what
-// replies could come from the server. They'll generally be things like
-// "PRIVMSG", "JOIN", etc. but all the numeric replies are left as ascii
-// strings of digits like "332" (mainly because I really didn't feel like
-// putting massive constant tables in).
-func (conn *Conn) AddHandler(name string, f IRCHandler) event.Handler {
-	h := NewHandler(f)
-	conn.ER.AddHandler(h, name)
-	return h
-}
-
-// Wrap f in an anonymous unboxing function
-func NewHandler(f IRCHandler) event.Handler {
-	return event.NewHandler(func(ev ...interface{}) {
-		f(ev[0].(*Conn), ev[1].(*Line))
-	})
-}
-
 // sets up the internal event handlers to do essential IRC protocol things
-var intHandlers map[string]event.Handler
-func init() {
-	intHandlers = make(map[string]event.Handler)
-	intHandlers["001"] = NewHandler((*Conn).h_001)
-	intHandlers["433"] = NewHandler((*Conn).h_433)
-	intHandlers["CTCP"] = NewHandler((*Conn).h_CTCP)
-	intHandlers["NICK"] = NewHandler((*Conn).h_NICK)
-	intHandlers["PING"] = NewHandler((*Conn).h_PING)
+var intHandlers = map[string]HandlerFunc{
+	"001": (*Conn).h_001,
+	"433": (*Conn).h_433,
+	"CTCP": (*Conn).h_CTCP,
+	"NICK": (*Conn).h_NICK,
+	"PING": (*Conn).h_PING,
 }
 
 func (conn *Conn) addIntHandlers() {
 	for n, h := range intHandlers {
-		conn.ER.AddHandler(h, n)
+		// internal handlers are essential for the IRC client
+		// to function, so we don't save their Removers here
+		conn.Handle(n, h)
 	}
 }
 
@@ -57,7 +32,7 @@ func (conn *Conn) h_PING(line *Line) {
 // Handler to trigger a "CONNECTED" event on receipt of numeric 001
 func (conn *Conn) h_001(line *Line) {
 	// we're connected!
-	conn.ED.Dispatch("connected", conn, line)
+	conn.dispatch(&Line{Cmd: "connected"})
 	// and we're being given our hostname (from the server's perspective)
 	t := line.Args[len(line.Args)-1]
 	if idx := strings.LastIndex(t, " "); idx != -1 {
@@ -108,3 +83,35 @@ func (conn *Conn) h_NICK(line *Line) {
 		conn.Me.Nick = line.Args[0]
 	}
 }
+
+// Handle PRIVMSGs that trigger Commands
+func (conn *Conn) h_PRIVMSG(line *Line) {
+	txt := line.Args[1]
+	if conn.CommandStripNick && strings.HasPrefix(txt, conn.Me.Nick) {
+		// Look for '^${nick}[:;>,-]? '
+		l := len(conn.Me.Nick)
+		switch txt[l] {
+		case ':', ';', '>', ',', '-':
+			l++
+		}
+		if txt[l] == ' ' {
+			txt = strings.TrimSpace(txt[l:])
+		}
+	}
+	cmd, l := conn.cmdMatch(txt)
+	if cmd == nil { return }
+	if conn.CommandStripPrefix {
+		txt = strings.TrimSpace(txt[l:])
+	}
+	if txt != line.Args[1] {
+		line = line.Copy()
+		line.Args[1] = txt
+	}
+	cmd.Execute(conn, line)
+}
+
+func (conn *Conn) c_HELP(line *Line) {
+	if cmd, _ := conn.cmdMatch(line.Args[1]); cmd != nil {
+		conn.Privmsg(line.Args[0], cmd.Help())
+	}
+}

client/handlers_test.go

@@ -2,8 +2,10 @@ package client
 
 import (
 	"code.google.com/p/gomock/gomock"
+	"fmt"
 	"github.com/fluffle/goirc/state"
 	"testing"
+	"time"
 )
 
 // This test performs a simple end-to-end verification of correct line parsing
@@ -11,14 +13,10 @@ import (
 // in this file will call their respective handlers synchronously, otherwise
 // testing becomes more difficult.
 func TestPING(t *testing.T) {
-	c, s := setUp(t)
+	_, s := setUp(t)
 	defer s.tearDown()
-	// As this is a real end-to-end test, we need a real end-to-end dispatcher.
-	c.ED = c.ER
 	s.nc.Send("PING :1234567890")
 	s.nc.Expect("PONG :1234567890")
-	// Return mock dispatcher to it's rightful place afterwards for tearDown.
-	c.ED = s.ed
 }
 
 // Test the handler for 001 / RPL_WELCOME
@@ -27,9 +25,18 @@ func Test001(t *testing.T) {
 	defer s.tearDown()
 
 	l := parseLine(":irc.server.org 001 test :Welcome to IRC test!ident@somehost.com")
-	s.ed.EXPECT().Dispatch("connected", c, l)
+	// Set up a handler to detect whether connected handler is called from 001
+	hcon := false
+	c.HandleFunc("connected", func (conn *Conn, line *Line) {
+		hcon = true
+	})
+
 	// Call handler with a valid 001 line
 	c.h_001(l)
+	<-time.After(time.Millisecond)
+	if !hcon {
+		t.Errorf("001 handler did not dispatch connected event.")
+	}
 
 	// Check host parsed correctly
 	if c.Me.Host != "somehost.com" {
@@ -132,6 +139,58 @@ func TestCTCP(t *testing.T) {
 	c.h_CTCP(parseLine(":blah!moo@cows.com PRIVMSG test :\001UNKNOWN ctcp\001"))
 }
 
+func TestPRIVMSG(t *testing.T){
+	c, s := setUp(t)
+	defer s.tearDown()
+
+	f := func (conn *Conn, line *Line) {
+		conn.Privmsg(line.Args[0], line.Args[1])
+	}
+	c.CommandFunc("prefix", f, "")
+
+	// CommandStripNick and CommandStripPrefix are both false to begin
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar"))
+	s.nc.Expect("PRIVMSG #foo :prefix bar")
+	// If we're not stripping off the nick, the prefix won't match.
+	// This might be considered a bug, but then the library currently has a
+	// poor understanding of the concept of "being addressed".
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar"))
+	s.nc.ExpectNothing()
+
+	c.CommandStripNick = true
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar"))
+	s.nc.Expect("PRIVMSG #foo :prefix bar")
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar"))
+	s.nc.Expect("PRIVMSG #foo :prefix bar")
+
+	c.CommandStripPrefix = true
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar"))
+	s.nc.Expect("PRIVMSG #foo :bar")
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar"))
+	s.nc.Expect("PRIVMSG #foo :bar")
+
+	c.CommandStripNick = false
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar"))
+	s.nc.Expect("PRIVMSG #foo :bar")
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar"))
+	s.nc.ExpectNothing()
+
+	// Check the various nick addressing notations that are supported.
+	c.CommandStripNick = true
+	for _, addr := range []string{":", ";", ",", ">", "-", ""} {
+		c.h_PRIVMSG(parseLine(fmt.Sprintf(
+			":blah!moo@cows.com PRIVMSG #foo :test%s prefix bar", addr)))
+		s.nc.Expect("PRIVMSG #foo :bar")
+		c.h_PRIVMSG(parseLine(fmt.Sprintf(
+			":blah!moo@cows.com PRIVMSG #foo :test%sprefix bar", addr)))
+		s.nc.ExpectNothing()
+	}
+	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test! prefix bar"))
+	s.nc.ExpectNothing()
+
+
+}
+
 // Test the handler for JOIN messages
 func TestJOIN(t *testing.T) {
 	c, s := setUp(t)

client/mocknetconn_test.go

@@ -74,7 +74,7 @@ func (m *mockNetConn) Send(s string) {
 
 func (m *mockNetConn) Expect(e string) {
 	select {
-	case <-time.After(1e6):
+	case <-time.After(time.Millisecond):
 		m.Errorf("Mock connection did not receive expected output.\n\t"+
 			"Expected: '%s', got nothing.", e)
 	case s := <-m.Out:
@@ -88,7 +88,7 @@ func (m *mockNetConn) Expect(e string) {
 
 func (m *mockNetConn) ExpectNothing() {
 	select {
-	case <-time.After(1e6):
+	case <-time.After(time.Millisecond):
 	case s := <-m.Out:
 		s = strings.Trim(s, "\r\n")
 		m.Errorf("Mock connection received unexpected output.\n\t"+

client/state_handlers.go

@@ -4,40 +4,37 @@ package client
 // to manage tracking state for an IRC connection
 
 import (
-	"github.com/fluffle/goevent/event"
 	"github.com/fluffle/golog/logging"
 	"strings"
 )
 
-var stHandlers map[string]event.Handler
-
-func init() {
-	stHandlers = make(map[string]event.Handler)
-	stHandlers["JOIN"] = NewHandler((*Conn).h_JOIN)
-	stHandlers["KICK"] = NewHandler((*Conn).h_KICK)
-	stHandlers["MODE"] = NewHandler((*Conn).h_MODE)
-	stHandlers["NICK"] = NewHandler((*Conn).h_STNICK)
-	stHandlers["PART"] = NewHandler((*Conn).h_PART)
-	stHandlers["QUIT"] = NewHandler((*Conn).h_QUIT)
-	stHandlers["TOPIC"] = NewHandler((*Conn).h_TOPIC)
-	stHandlers["311"] = NewHandler((*Conn).h_311)
-	stHandlers["324"] = NewHandler((*Conn).h_324)
-	stHandlers["332"] = NewHandler((*Conn).h_332)
-	stHandlers["352"] = NewHandler((*Conn).h_352)
-	stHandlers["353"] = NewHandler((*Conn).h_353)
-	stHandlers["671"] = NewHandler((*Conn).h_671)
+var stHandlers = map[string]HandlerFunc{
+	"JOIN": (*Conn).h_JOIN,
+	"KICK": (*Conn).h_KICK,
+	"MODE": (*Conn).h_MODE,
+	"NICK": (*Conn).h_STNICK,
+	"PART": (*Conn).h_PART,
+	"QUIT": (*Conn).h_QUIT,
+	"TOPIC": (*Conn).h_TOPIC,
+	"311": (*Conn).h_311,
+	"324": (*Conn).h_324,
+	"332": (*Conn).h_332,
+	"352": (*Conn).h_352,
+	"353": (*Conn).h_353,
+	"671": (*Conn).h_671,
 }
 
 func (conn *Conn) addSTHandlers() {
 	for n, h := range stHandlers {
-		conn.ER.AddHandler(h, n)
+		conn.stRemovers = append(conn.stRemovers, conn.Handle(n, h))
 	}
 }
 
 func (conn *Conn) delSTHandlers() {
-	for n, h := range stHandlers {
-		conn.ER.DelHandler(h, n)
+	for _, h := range conn.stRemovers {
+		h.Remove()
 	}
+	conn.stRemovers = conn.stRemovers[:0]
 }
 
 // Handle NICK messages that need to update the state tracker