Merge pull request #664 from slingamn/sort_defs.2

tweaks to cap listing

gencapdefs.py

@@ -180,6 +180,7 @@ CAPDEFS = [
 ]
 
 def validate_defs():
+    CAPDEFS.sort(key=lambda d: d.name)
     numCaps = len(CAPDEFS)
     numNames = len(set(capdef.name for capdef in CAPDEFS))
     if numCaps != numNames:

irc/caps/defs.go

@@ -13,10 +13,6 @@ const (
 )
 
 const (
-	// Acc is the proposed IRCv3 capability named "draft/acc":
-	// https://github.com/ircv3/ircv3-specifications/pull/276
-	Acc Capability = iota
-
 	// AccountNotify is the IRCv3 capability named "account-notify":
 	// https://ircv3.net/specs/extensions/account-notify-3.1.html
 	AccountNotify Capability = iota
@@ -41,6 +37,34 @@ const (
 	// https://ircv3.net/specs/extensions/chghost-3.2.html
 	ChgHost Capability = iota
 
+	// Acc is the proposed IRCv3 capability named "draft/acc":
+	// https://github.com/ircv3/ircv3-specifications/pull/276
+	Acc Capability = iota
+
+	// EventPlayback is the Proposed IRCv3 capability named "draft/event-playback":
+	// https://github.com/ircv3/ircv3-specifications/pull/362
+	EventPlayback Capability = iota
+
+	// LabeledResponse is the draft IRCv3 capability named "draft/labeled-response-0.2":
+	// https://ircv3.net/specs/extensions/labeled-response.html
+	LabeledResponse Capability = iota
+
+	// Languages is the proposed IRCv3 capability named "draft/languages":
+	// https://gist.github.com/DanielOaks/8126122f74b26012a3de37db80e4e0c6
+	Languages Capability = iota
+
+	// Rename is the proposed IRCv3 capability named "draft/rename":
+	// https://github.com/SaberUK/ircv3-specifications/blob/rename/extensions/rename.md
+	Rename Capability = iota
+
+	// Resume is the proposed IRCv3 capability named "draft/resume-0.5":
+	// https://github.com/DanielOaks/ircv3-specifications/blob/master+resume/extensions/resume.md
+	Resume Capability = iota
+
+	// SetName is the proposed IRCv3 capability named "draft/setname":
+	// https://github.com/ircv3/ircv3-specifications/pull/361
+	SetName Capability = iota
+
 	// EchoMessage is the IRCv3 capability named "echo-message":
 	// https://ircv3.net/specs/extensions/echo-message-3.2.html
 	EchoMessage Capability = iota
@@ -53,18 +77,6 @@ const (
 	// https://ircv3.net/specs/extensions/invite-notify-3.2.html
 	InviteNotify Capability = iota
 
-	// LabeledResponse is the draft IRCv3 capability named "draft/labeled-response-0.2":
-	// https://ircv3.net/specs/extensions/labeled-response.html
-	LabeledResponse Capability = iota
-
-	// Languages is the proposed IRCv3 capability named "draft/languages":
-	// https://gist.github.com/DanielOaks/8126122f74b26012a3de37db80e4e0c6
-	Languages Capability = iota
-
-	// MaxLine is the Oragono-specific capability named "oragono.io/maxline-2":
-	// https://oragono.io/maxline-2
-	MaxLine Capability = iota
-
 	// MessageTags is the IRCv3 capability named "message-tags":
 	// https://ircv3.net/specs/extensions/message-tags.html
 	MessageTags Capability = iota
@@ -73,13 +85,17 @@ const (
 	// https://ircv3.net/specs/extensions/multi-prefix-3.1.html
 	MultiPrefix Capability = iota
 
-	// Rename is the proposed IRCv3 capability named "draft/rename":
-	// https://github.com/SaberUK/ircv3-specifications/blob/rename/extensions/rename.md
-	Rename Capability = iota
+	// Bouncer is the Oragono-specific capability named "oragono.io/bnc":
+	// https://oragono.io/bnc
+	Bouncer Capability = iota
 
-	// Resume is the proposed IRCv3 capability named "draft/resume-0.5":
-	// https://github.com/DanielOaks/ircv3-specifications/blob/master+resume/extensions/resume.md
-	Resume Capability = iota
+	// MaxLine is the Oragono-specific capability named "oragono.io/maxline-2":
+	// https://oragono.io/maxline-2
+	MaxLine Capability = iota
+
+	// Nope is the Oragono vendor capability named "oragono.io/nope":
+	// https://oragono.io/nope
+	Nope Capability = iota
 
 	// SASL is the IRCv3 capability named "sasl":
 	// https://ircv3.net/specs/extensions/sasl-3.2.html
@@ -89,10 +105,6 @@ const (
 	// https://ircv3.net/specs/extensions/server-time-3.2.html
 	ServerTime Capability = iota
 
-	// SetName is the proposed IRCv3 capability named "draft/setname":
-	// https://github.com/ircv3/ircv3-specifications/pull/361
-	SetName Capability = iota
-
 	// STS is the IRCv3 capability named "sts":
 	// https://ircv3.net/specs/extensions/sts.html
 	STS Capability = iota
@@ -101,56 +113,44 @@ const (
 	// https://ircv3.net/specs/extensions/userhost-in-names-3.2.html
 	UserhostInNames Capability = iota
 
-	// Bouncer is the Oragono-specific capability named "oragono.io/bnc":
-	// https://oragono.io/bnc
-	Bouncer Capability = iota
-
-	// ZNCSelfMessage is the ZNC vendor capability named "znc.in/self-message":
-	// https://wiki.znc.in/Query_buffers
-	ZNCSelfMessage Capability = iota
-
-	// EventPlayback is the Proposed IRCv3 capability named "draft/event-playback":
-	// https://github.com/ircv3/ircv3-specifications/pull/362
-	EventPlayback Capability = iota
-
 	// ZNCPlayback is the ZNC vendor capability named "znc.in/playback":
 	// https://wiki.znc.in/Playback
 	ZNCPlayback Capability = iota
 
-	// Nope is the Oragono vendor capability named "oragono.io/nope":
-	// https://oragono.io/nope
-	Nope Capability = iota
+	// ZNCSelfMessage is the ZNC vendor capability named "znc.in/self-message":
+	// https://wiki.znc.in/Query_buffers
+	ZNCSelfMessage Capability = iota
 )
 
 // `capabilityNames[capab]` is the string name of the capability `capab`
 var (
 	capabilityNames = [numCapabs]string{
-		"draft/acc",
 		"account-notify",
 		"account-tag",
 		"away-notify",
 		"batch",
 		"cap-notify",
 		"chghost",
+		"draft/acc",
+		"draft/event-playback",
+		"draft/labeled-response-0.2",
+		"draft/languages",
+		"draft/rename",
+		"draft/resume-0.5",
+		"draft/setname",
 		"echo-message",
 		"extended-join",
 		"invite-notify",
-		"draft/labeled-response-0.2",
-		"draft/languages",
-		"oragono.io/maxline-2",
 		"message-tags",
 		"multi-prefix",
-		"draft/rename",
-		"draft/resume-0.5",
+		"oragono.io/bnc",
+		"oragono.io/maxline-2",
+		"oragono.io/nope",
 		"sasl",
 		"server-time",
-		"draft/setname",
 		"sts",
 		"userhost-in-names",
-		"oragono.io/bnc",
-		"znc.in/self-message",
-		"draft/event-playback",
 		"znc.in/playback",
-		"oragono.io/nope",
+		"znc.in/self-message",
 	}
 )

irc/caps/set.go

@@ -4,9 +4,7 @@
 package caps
 
 import (
-	"bytes"
-	"sort"
-
+	"fmt"
 	"github.com/oragono/oragono/irc/utils"
 )
 
@@ -91,11 +89,15 @@ func (s *Set) Empty() bool {
 	return utils.BitsetEmpty(s[:])
 }
 
-const maxPayloadLength = 440
+const defaultMaxPayloadLength = 450
 
 // Strings returns all of our enabled capabilities as a slice of strings.
-func (s *Set) Strings(version Version, values Values) (result []string) {
-	var strs sort.StringSlice
+func (s *Set) Strings(version Version, values Values, maxLen int) (result []string) {
+	if maxLen == 0 {
+		maxLen = defaultMaxPayloadLength
+	}
+	var t utils.TokenLineBuilder
+	t.Initialize(maxLen, " ")
 
 	var capab Capability
 	asSlice := s[:]
@@ -108,37 +110,15 @@ func (s *Set) Strings(version Version, values Values) (result []string) {
 		if version >= Cap302 {
 			val, exists := values[capab]
 			if exists {
-				capString += "=" + val
+				capString = fmt.Sprintf("%s=%s", capString, val)
 			}
 		}
-		strs = append(strs, capString)
+		t.Add(capString)
 	}
 
-	if len(strs) == 0 {
-		return []string{""}
-	}
-
-	// sort the cap string before we send it out
-	sort.Sort(strs)
-
-	var buf bytes.Buffer
-	for _, str := range strs {
-		tokenLen := len(str)
-		if buf.Len() != 0 {
-			tokenLen += 1
-		}
-		if maxPayloadLength < buf.Len()+tokenLen {
-			result = append(result, buf.String())
-			buf.Reset()
-		}
-		if buf.Len() != 0 {
-			buf.WriteByte(' ')
-		}
-		buf.WriteString(str)
+	result = t.Lines()
+	if result == nil {
+		result = []string{""}
 	}
-	if buf.Len() != 0 {
-		result = append(result, buf.String())
-	}
-
 	return
 }

irc/caps/set_test.go

@@ -47,13 +47,13 @@ func TestSets(t *testing.T) {
 	values := make(Values)
 	values[InviteNotify] = "invitemepls"
 
-	actualCap301ValuesString := s1.Strings(Cap301, values)
+	actualCap301ValuesString := s1.Strings(Cap301, values, 0)
 	expectedCap301ValuesString := []string{"invite-notify userhost-in-names"}
 	if !reflect.DeepEqual(actualCap301ValuesString, expectedCap301ValuesString) {
 		t.Errorf("Generated Cap301 values string [%v] did not match expected values string [%v]", actualCap301ValuesString, expectedCap301ValuesString)
 	}
 
-	actualCap302ValuesString := s1.Strings(Cap302, values)
+	actualCap302ValuesString := s1.Strings(Cap302, values, 0)
 	expectedCap302ValuesString := []string{"invite-notify=invitemepls userhost-in-names"}
 	if !reflect.DeepEqual(actualCap302ValuesString, expectedCap302ValuesString) {
 		t.Errorf("Generated Cap302 values string [%s] did not match expected values string [%s]", actualCap302ValuesString, expectedCap302ValuesString)

irc/handlers.go

@@ -569,9 +569,14 @@ func capHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 
 	sendCapLines := func(cset *caps.Set, values caps.Values) {
 		version := rb.session.capVersion
-		capLines := cset.Strings(version, values)
-		// weechat 1.4 has a bug here where it won't accept the CAP reply unless it contains
-		// the server.name source:
+		// we're working around two bugs:
+		// 1. weechat 1.4 won't accept the CAP reply unless it contains the server.name source
+		// 2. old versions of Kiwi and The Lounge can't parse multiline CAP LS 302 (#661),
+		// so try as hard as possible to get the response to fit on one line.
+		// :server.name CAP * LS * :<tokens>
+		// 1           7         4
+		maxLen := 510 - 1 - len(server.name) - 7 - len(subCommand) - 4
+		capLines := cset.Strings(version, values, maxLen)
 		for i, capStr := range capLines {
 			if version >= caps.Cap302 && i < len(capLines)-1 {
 				rb.Add(nil, server.name, "CAP", details.nick, subCommand, "*", capStr)

irc/server.go

@@ -710,17 +710,17 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 
 	// updated caps get DEL'd and then NEW'd
 	// so, we can just add updated ones to both removed and added lists here and they'll be correctly handled
-	server.logger.Debug("server", "Updated Caps", strings.Join(updatedCaps.Strings(caps.Cap301, config.Server.capValues), " "))
+	server.logger.Debug("server", "Updated Caps", strings.Join(updatedCaps.Strings(caps.Cap301, config.Server.capValues, 0), " "))
 	addedCaps.Union(updatedCaps)
 	removedCaps.Union(updatedCaps)
 
 	if !addedCaps.Empty() || !removedCaps.Empty() {
 		capBurstSessions = server.clients.AllWithCapsNotify()
 
-		added[caps.Cap301] = addedCaps.Strings(caps.Cap301, config.Server.capValues)
-		added[caps.Cap302] = addedCaps.Strings(caps.Cap302, config.Server.capValues)
+		added[caps.Cap301] = addedCaps.Strings(caps.Cap301, config.Server.capValues, 0)
+		added[caps.Cap302] = addedCaps.Strings(caps.Cap302, config.Server.capValues, 0)
 		// removed never has values, so we leave it as Cap301
-		removed = removedCaps.Strings(caps.Cap301, config.Server.capValues)
+		removed = removedCaps.Strings(caps.Cap301, config.Server.capValues, 0)
 	}
 
 	for _, sSession := range capBurstSessions {

irc/utils/text.go

@@ -83,3 +83,44 @@ func MakeSplitMessage(original string, origIs512 bool) (result SplitMessage) {
 
 	return
 }
+
+// TokenLineBuilder is a helper for building IRC lines composed of delimited tokens,
+// with a maximum line length.
+type TokenLineBuilder struct {
+	lineLen int
+	delim   string
+	buf     bytes.Buffer
+	result  []string
+}
+
+func (t *TokenLineBuilder) Initialize(lineLen int, delim string) {
+	t.lineLen = lineLen
+	t.delim = delim
+}
+
+// Add adds a token to the line, creating a new line if necessary.
+func (t *TokenLineBuilder) Add(token string) {
+	tokenLen := len(token)
+	if t.buf.Len() != 0 {
+		tokenLen += len(t.delim)
+	}
+	if t.lineLen < t.buf.Len()+tokenLen {
+		t.result = append(t.result, t.buf.String())
+		t.buf.Reset()
+	}
+	if t.buf.Len() != 0 {
+		t.buf.WriteString(t.delim)
+	}
+	t.buf.WriteString(token)
+}
+
+// Lines terminates the line-building and returns all the lines.
+func (t *TokenLineBuilder) Lines() (result []string) {
+	result = t.result
+	t.result = nil
+	if t.buf.Len() != 0 {
+		result = append(result, t.buf.String())
+		t.buf.Reset()
+	}
+	return
+}

irc/utils/text_test.go

@@ -58,3 +58,27 @@ func BenchmarkWordWrap(b *testing.B) {
 		WordWrap(monteCristo, 60)
 	}
 }
+
+func TestTokenLineBuilder(t *testing.T) {
+	lineLen := 400
+	var tl TokenLineBuilder
+	tl.Initialize(lineLen, " ")
+	for _, token := range strings.Fields(monteCristo) {
+		tl.Add(token)
+	}
+
+	lines := tl.Lines()
+	if len(lines) != 4 {
+		t.Errorf("expected 4 lines, got %d", len(lines))
+	}
+	for _, line := range lines {
+		if len(line) > lineLen {
+			t.Errorf("line length %d exceeds maximum of %d", len(line), lineLen)
+		}
+	}
+
+	joined := strings.Join(lines, " ")
+	if joined != monteCristo {
+		t.Errorf("text incorrectly split into lines: %s instead of %s", joined, monteCristo)
+	}
+}