Handlers/Commands now use the container/list.

Re introduced strip nick and strip prefix for SimpleCommands
Fixed tests.

client/connection.go

@@ -21,7 +21,7 @@ type Conn struct {
 	password string
 
 	// Handlers and Commands
-	handlers *hSet
+	handlers *handlerSet
 	commands *commandList
 
 	// State tracker for nicks and channels
@@ -54,6 +54,9 @@ type Conn struct {
 	// Client->server ping frequency, in seconds. Defaults to 3m.
 	PingFreq time.Duration
 
+	// Controls what is stripped from line.Args[1] for Commands
+	CommandStripNick, SimpleCommandStripPrefix bool
+
 	// Set this to true to disable flood protection and false to re-enable
 	Flood bool
 
@@ -81,7 +84,7 @@ func Client(nick string, args ...string) *Conn {
 		cSend:      make(chan bool),
 		cLoop:      make(chan bool),
 		cPing:      make(chan bool),
-		handlers:   handlerSet(),
+		handlers:   newHandlerSet(),
 		commands:   newCommandList(),
 		stRemovers: make([]Remover, 0, len(stHandlers)),
 		PingFreq:   3 * time.Minute,

client/dispatch.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"container/list"
 	"fmt"
 	"github.com/fluffle/golog/logging"
 	"math"
@@ -31,163 +32,119 @@ func (r RemoverFunc) Remove() {
 	r()
 }
 
-type hList struct {
+type handlerElement struct {
-	start, end *hNode
-}
-
-type hNode struct {
-	next, prev *hNode
-	set        *hSet
 	event   string
 	handler Handler
 }
 
-func (hn *hNode) Handle(conn *Conn, line *Line) {
+type handlerSet struct {
-	hn.handler.Handle(conn, line)
+	set map[string]*list.List
-}
-
-func (hn *hNode) Remove() {
-	hn.set.remove(hn)
-}
-
-type hSet struct {
-	set map[string]*hList
 	sync.RWMutex
 }
 
-func handlerSet() *hSet {
+func newHandlerSet() *handlerSet {
-	return &hSet{set: make(map[string]*hList)}
+	return &handlerSet{set: make(map[string]*list.List)}
 }
 
-func (hs *hSet) add(ev string, h Handler) Remover {
+func (hs *handlerSet) add(event string, handler Handler) Remover {
 	hs.Lock()
 	defer hs.Unlock()
-	ev = strings.ToLower(ev)
+	event = strings.ToLower(event)
-	l, ok := hs.set[ev]
+	l, ok := hs.set[event]
 	if !ok {
-		l = &hList{}
+		l = list.New()
+		hs.set[event] = l
 	}
-	hn := &hNode{
+	element := l.PushBack(&handlerElement{event, handler})
-		set:     hs,
+	return RemoverFunc(func() {
-		event:   ev,
+		hs.remove(element)
-		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) {
+func (hs *handlerSet) remove(element *list.Element) {
 	hs.Lock()
 	defer hs.Unlock()
-	l, ok := hs.set[hn.event]
+	h := element.Value.(*handlerElement)
+	l, ok := hs.set[h.event]
 	if !ok {
-		logging.Error("Removing node for unknown event '%s'", hn.event)
+		logging.Error("Removing node for unknown event '%s'", h.event)
 		return
 	}
-	if hn.next == nil {
+	l.Remove(element)
-		l.end = hn.prev
+	if l.Len() == 0 {
-	} else {
+		delete(hs.set, h.event)
-		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) {
+func (hs *handlerSet) dispatch(conn *Conn, line *Line) {
 	hs.RLock()
 	defer hs.RUnlock()
-	ev := strings.ToLower(line.Cmd)
+	event := strings.ToLower(line.Cmd)
-	list, ok := hs.set[ev]
+	l, ok := hs.set[event]
 	if !ok {
 		return
 	}
-	for hn := list.start; hn != nil; hn = hn.next {
+
-		go hn.Handle(conn, line)
+	for e := l.Front(); e != nil; e = e.Next() {
+		h := e.Value.(*handlerElement)
+		go h.handler.Handle(conn, line)
 	}
 }
 
-type command struct {
+type commandElement struct {
-	handler  Handler
-	set      *commandList
 	regex    string
+	handler  Handler
 	priority int
 }
 
-func (c *command) Handle(conn *Conn, line *Line) {
-	c.handler.Handle(conn, line)
-}
-
-func (c *command) Remove() {
-	c.set.remove(c)
-}
-
 type commandList struct {
-	set []*command
+	list *list.List
 	sync.RWMutex
 }
 
 func newCommandList() *commandList {
-	return &commandList{}
+	return &commandList{list: list.New()}
 }
 
 func (cl *commandList) add(regex string, handler Handler, priority int) Remover {
 	cl.Lock()
 	defer cl.Unlock()
-	c := &command{
+	c := &commandElement{
-		handler:  handler,
-		set:      cl,
 		regex:    regex,
+		handler:  handler,
 		priority: priority,
 	}
 	// Check for exact regex matches. This will filter out any repeated SimpleCommands.
-	for _, c := range cl.set {
+	for e := cl.list.Front(); e != nil; e = e.Next() {
+		c := e.Value.(*commandElement)
 		if c.regex == regex {
 			logging.Error("Command prefix '%s' already registered.", regex)
 			return nil
 		}
 	}
-	cl.set = append(cl.set, c)
+	element := cl.list.PushBack(c)
-	return c
+	return RemoverFunc(func() {
+		cl.remove(element)
+	})
 }
 
-func (cl *commandList) remove(c *command) {
+func (cl *commandList) remove(element *list.Element) {
 	cl.Lock()
 	defer cl.Unlock()
-	for index, value := range cl.set {
+	cl.list.Remove(element)
-		if value == c {
-			copy(cl.set[index:], cl.set[index+1:])
-			cl.set = cl.set[:len(cl.set)-1]
-			c.set = nil
-			return
-		}
-	}
 }
 
 // Matches the command with the highest priority.
-func (cl *commandList) match(txt string) (handler Handler) {
+func (cl *commandList) match(text string) (handler Handler) {
 	cl.RLock()
 	defer cl.RUnlock()
 	maxPriority := math.MinInt32
-	for _, c := range cl.set {
+	text = strings.ToLower(text)
+	for e := cl.list.Front(); e != nil; e = e.Next() {
+		c := e.Value.(*commandElement)
 		if c.priority > maxPriority {
 			if regex, error := regexp.Compile(c.regex); error == nil {
-				if regex.MatchString(txt) {
+				if regex.MatchString(text) {
 					maxPriority = c.priority
 					handler = c.handler
 				}
@@ -226,7 +183,18 @@ var SimpleCommandRegex string = `^!%v(\s|$)`
 // !roll
 // Because simple commands are simple, they get the highest priority.
 func (conn *Conn) SimpleCommand(prefix string, handler Handler) Remover {
-	return conn.Command(fmt.Sprintf(SimpleCommandRegex, strings.ToLower(prefix)), handler, math.MaxInt32)
+	stripHandler := func(conn *Conn, line *Line) {
+		text := line.Message()
+		if conn.SimpleCommandStripPrefix {
+			text = strings.TrimSpace(text[len(prefix):])
+		}
+		if text != line.Message() {
+			line = line.Copy()
+			line.Args[1] = text
+		}
+		handler.Handle(conn, line)
+	}
+	return conn.Command(fmt.Sprintf(SimpleCommandRegex, strings.ToLower(prefix)), HandlerFunc(stripHandler), math.MaxInt32)
 }
 
 func (conn *Conn) SimpleCommandFunc(prefix string, handlerFunc HandlerFunc) Remover {
@@ -256,10 +224,6 @@ func (conn *Conn) dispatch(line *Line) {
 	conn.handlers.dispatch(conn, line)
 }
 
-func (conn *Conn) command(line *Line) {
+func (conn *Conn) command(line *Line) Handler {
-	command := conn.commands.match(strings.ToLower(line.Message()))
+	return conn.commands.match(line.Message())
-	if command != nil {
-		command.Handle(conn, line)
-	}
-
 }

client/dispatch_test.go

@@ -6,7 +6,7 @@ import (
 )
 
 func TestHandlerSet(t *testing.T) {
-	hs := handlerSet()
+	hs := newHandlerSet()
 	if len(hs.set) != 0 {
 		t.Errorf("New set contains things!")
 	}
@@ -17,66 +17,40 @@ func TestHandlerSet(t *testing.T) {
 	}
 
 	// Add one
-	hn1 := hs.add("ONE", HandlerFunc(f)).(*hNode)
+	hn1 := hs.add("ONE", HandlerFunc(f))
 	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 {
+	if hl.Len() != 1 {
-		t.Errorf("First node for 'one' not created correctly")
+		t.Errorf("List doesn't contain 'one' after add().")
-	}
-	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)
+	hn2 := hs.add("one", HandlerFunc(f))
 	if len(hs.set) != 1 {
 		t.Errorf("Set contains more than 'one' list after add().")
 	}
-	if hn2.set != hs || hn2.event != "one" {
+	if hl.Len() != 2 {
-		t.Errorf("Second node for 'one' not created correctly")
+		t.Errorf("List doesn't contain second 'one' after add().")
-	}
-	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)
+	hn3 := hs.add("one", HandlerFunc(f))
 	if len(hs.set) != 1 {
 		t.Errorf("Set contains more than 'one' list after add().")
 	}
-	if hn3.set != hs || hn3.event != "one" {
+	if hl.Len() != 3 {
-		t.Errorf("Third node for 'one' not created correctly")
+		t.Errorf("List doesn't contain third 'one' after add().")
-	}
-	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)
+	hn4 := hs.add("one", HandlerFunc(f))
 	if len(hs.set) != 1 {
 		t.Errorf("Set contains more than 'one' list after add().")
 	}
-	if hn4.set != hs || hn4.event != "one" {
+	if hl.Len() != 4 {
-		t.Errorf("Fourth node for 'one' not created correctly.")
+		t.Errorf("List doesn't contain fourth 'one' after add().")
-	}
-	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.
@@ -94,16 +68,8 @@ func TestHandlerSet(t *testing.T) {
 	if len(hs.set) != 1 {
 		t.Errorf("Set list count changed after remove().")
 	}
-	if hn3.set != nil || hn3.prev != nil || hn3.next != nil {
+	if hl.Len() != 3 {
-		t.Errorf("Third node for 'one' not removed correctly.")
+		t.Errorf("Third '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.
@@ -114,18 +80,12 @@ func TestHandlerSet(t *testing.T) {
 	}
 
 	// Remove node 1.
-	hs.remove(hn1)
+	hn1.Remove()
 	if len(hs.set) != 1 {
 		t.Errorf("Set list count changed after remove().")
 	}
-	if hn1.set != nil || hn1.prev != nil || hn1.next != nil {
+	if hl.Len() != 2 {
-		t.Errorf("First node for 'one' not removed correctly.")
+		t.Errorf("First '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.
@@ -140,14 +100,8 @@ func TestHandlerSet(t *testing.T) {
 	if len(hs.set) != 1 {
 		t.Errorf("Set list count changed after remove().")
 	}
-	if hn4.set != nil || hn4.prev != nil || hn4.next != nil {
+	if hl.Len() != 1 {
-		t.Errorf("Fourth node for 'one' not removed correctly.")
+		t.Errorf("Fourth '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.
@@ -158,16 +112,10 @@ func TestHandlerSet(t *testing.T) {
 	}
 
 	// Remove node 2.
-	hs.remove(hn2)
+	hn2.Remove()
 	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"})
@@ -178,52 +126,43 @@ func TestHandlerSet(t *testing.T) {
 }
 
 func TestCommandSet(t *testing.T) {
-	cs := commandSet()
+	cl := newCommandList()
-	if len(cs.set) != 0 {
+	if cl.list.Len() != 0 {
-		t.Errorf("New set contains things!")
+		t.Errorf("New list contains things!")
 	}
 
-	c := &command{
+	cn1 := cl.add("one", HandlerFunc(func(c *Conn, l *Line) {}), 0)
-		fn:   func(c *Conn, l *Line) {},
+	if cl.list.Len() != 1 {
-		help: "wtf?",
+		t.Errorf("Command 'one' not added to list correctly.")
 	}
 
-	cn1 := cs.add("ONE", c).(*cNode)
+	cn2 := cl.add("one two", HandlerFunc(func(c *Conn, l *Line) {}), 0)
-	if _, ok := cs.set["one"]; !ok || cn1.set != cs || cn1.prefix != "one" {
+	if cl.list.Len() != 2 {
-		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 {
+	if c := cl.match("foo"); c != nil {
 		t.Errorf("Matched 'foo' when we shouldn't.")
 	}
-	if c, l := cs.match("one"); c.(*cNode) != cn1 || l != 3 {
+	if c := cl.match("one"); c == nil {
-		t.Errorf("Didn't match 'one' when we should have.")
+		t.Errorf("Didn't match when we should have.")
 	}
-	if c, l := cs.match("one two three"); c.(*cNode) != cn2 || l != 7 {
+	if c := cl.match("one two three"); c == nil {
-		t.Errorf("Didn't match 'one two' when we should have.")
+		t.Errorf("Didn't match when we should have.")
 	}
 
-	cs.remove(cn2)
+	cn2.Remove()
-	if _, ok := cs.set["one two"]; ok || cn2.set != nil {
+	if cl.list.Len() != 1 {
 		t.Errorf("Command 'one two' not removed correctly.")
 	}
-	if c, l := cs.match("one two three"); c.(*cNode) != cn1 || l != 3 {
+	if c := cl.match("one two three"); c == nil {
-		t.Errorf("Didn't match 'one' when we should have.")
+		t.Errorf("Didn't match when we should have.")
 	}
 	cn1.Remove()
-	if _, ok := cs.set["one"]; ok || cn1.set != nil {
+	if cl.list.Len() != 0 {
-		t.Errorf("Command 'one' not removed correctly.")
+		t.Errorf("Command 'one two' not removed correctly.")
 	}
-	if c, l := cs.match("one two three"); c != nil || l != 0 {
+	if c := cl.match("one two three"); c != nil {
-		t.Errorf("Matched 'one' when we shouldn't have.")
+		t.Errorf("Matched 'one' when we shouldn't.")
 	}
 }

client/handlers.go

@@ -97,5 +97,23 @@ func (conn *Conn) h_NICK(line *Line) {
 
 // Handle PRIVMSGs that trigger Commands
 func (conn *Conn) h_PRIVMSG(line *Line) {
-	conn.command(line)
+	text := line.Message()
+	if conn.CommandStripNick && strings.HasPrefix(text, conn.Me.Nick) {
+		// Look for '^${nick}[:;>,-]? '
+		l := len(conn.Me.Nick)
+		switch text[l] {
+		case ':', ';', '>', ',', '-':
+			l++
+		}
+		if text[l] == ' ' {
+			text = strings.TrimSpace(text[l:])
+		}
+		line = line.Copy()
+		line.Args[1] = text
+	}
+	cmd := conn.command(line)
+	if cmd == nil {
+		return
+	}
+	cmd.Handle(conn, line)
 }

client/handlers_test.go

@@ -146,7 +146,9 @@ func TestPRIVMSG(t *testing.T) {
 	f := func(conn *Conn, line *Line) {
 		conn.Privmsg(line.Args[0], line.Args[1])
 	}
-	c.CommandFunc("prefix", f, "")
+	// Test legacy simpleCommands, with out the !.
+	SimpleCommandRegex = `^%v(\s|$)`
+	c.SimpleCommandFunc("prefix", f)
 
 	// CommandStripNick and CommandStripPrefix are both false to begin
 	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :prefix bar"))
@@ -163,7 +165,7 @@ func TestPRIVMSG(t *testing.T) {
 	c.h_PRIVMSG(parseLine(":blah!moo@cows.com PRIVMSG #foo :test: prefix bar"))
 	s.nc.Expect("PRIVMSG #foo :prefix bar")
 
-	c.CommandStripPrefix = true
+	c.SimpleCommandStripPrefix = 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"))