Merge pull request #2069 from slingamn/nestedbatch.1

some cleanups

irc/caps/constants.go

@@ -62,6 +62,8 @@ const (
 	RelaymsgTagName = "draft/relaymsg"
 	// BOT mode: https://ircv3.net/specs/extensions/bot-mode
 	BotTagName = "bot"
+	// https://ircv3.net/specs/extensions/chathistory
+	ChathistoryTargetsBatchType = "draft/chathistory-targets"
 )
 
 func init() {

irc/channel.go

@@ -1059,7 +1059,7 @@ func (channel *Channel) replayHistoryItems(rb *ResponseBuffer, items []history.I
 		}
 	}
 
-	batchID := rb.StartNestedHistoryBatch(chname)
+	batchID := rb.StartNestedBatch(chname, "chathistory")
 	defer rb.EndNestedBatch(batchID)
 
 	for _, item := range items {

irc/client.go

@@ -850,7 +850,7 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I
 	if target == "" {
 		target = nick
 	}
-	batchID = rb.StartNestedHistoryBatch(target)
+	batchID = rb.StartNestedBatch(target, "chathistory")
 
 	isSelfMessage := func(item *history.Item) bool {
 		// XXX: Params[0] is the message target. if the source of this message is an in-memory
@@ -1425,27 +1425,27 @@ func composeMultilineBatch(batchID, fromNickMask, fromAccount string, isBot bool
 }
 
 var (
-	// these are all the output commands that MUST have their last param be a trailing.
-	// this is needed because dumb clients like to treat trailing params separately from the
-	// other params in messages.
-	commandsThatMustUseTrailing = map[string]bool{
-		"PRIVMSG": true,
-		"NOTICE":  true,
-
-		RPL_WHOISCHANNELS: true,
-		RPL_USERHOST:      true,
-
+	// in practice, many clients require that the final parameter be a trailing
+	// (prefixed with `:`) even when this is not syntactically necessary.
+	// by default, force the following commands to use a trailing:
+	commandsThatMustUseTrailing = utils.SetLiteral(
+		"PRIVMSG",
+		"NOTICE",
+		RPL_WHOISCHANNELS,
+		RPL_USERHOST,
 		// mirc's handling of RPL_NAMREPLY is broken:
 		// https://forums.mirc.com/ubbthreads.php/topics/266939/re-nick-list
-		RPL_NAMREPLY: true,
-	}
+		RPL_NAMREPLY,
+	)
 )
 
+func forceTrailing(config *Config, command string) bool {
+	return config.Server.Compatibility.forceTrailing && commandsThatMustUseTrailing.Has(command)
+}
+
 // SendRawMessage sends a raw message to the client.
 func (session *Session) SendRawMessage(message ircmsg.Message, blocking bool) error {
-	// use dumb hack to force the last param to be a trailing param if required
-	config := session.client.server.Config()
-	if config.Server.Compatibility.forceTrailing && commandsThatMustUseTrailing[message.Command] {
+	if forceTrailing(session.client.server.Config(), message.Command) {
 		message.ForceTrailing()
 	}
 

irc/handlers.go

@@ -655,10 +655,8 @@ func chathistoryHandler(server *Server, client *Client, msg ircmsg.Message, rb *
 		} else {
 			// successful responses are sent as a chathistory or history batch
 			if listTargets {
-				if rb.session.capabilities.Has(caps.Batch) { // #2066
-					batchID := rb.StartNestedBatch("draft/chathistory-targets")
+				batchID := rb.StartNestedBatch(caps.ChathistoryTargetsBatchType)
 				defer rb.EndNestedBatch(batchID)
-				}
 				for _, target := range targets {
 					name := server.UnfoldName(target.CfName)
 					rb.Add(nil, server.name, "CHATHISTORY", "TARGETS", name,

irc/message_cache.go

@@ -79,8 +79,7 @@ func (m *MessageCache) Initialize(server *Server, serverTime time.Time, msgid st
 	m.params = params
 
 	var msg ircmsg.Message
-	config := server.Config()
-	if config.Server.Compatibility.forceTrailing && commandsThatMustUseTrailing[command] {
+	if forceTrailing(server.Config(), command) {
 		msg.ForceTrailing()
 	}
 	msg.Source = nickmask
@@ -111,8 +110,7 @@ func (m *MessageCache) InitializeSplitMessage(server *Server, nickmask, accountN
 	m.target = target
 	m.splitMessage = message
 
-	config := server.Config()
-	forceTrailing := config.Server.Compatibility.forceTrailing && commandsThatMustUseTrailing[command]
+	forceTrailing := forceTrailing(server.Config(), command)
 
 	if message.Is512() {
 		isTagmsg := command == "TAGMSG"

irc/responsebuffer.go

@@ -193,6 +193,9 @@ func (rb *ResponseBuffer) sendBatchEnd(blocking bool) {
 // Starts a nested batch (see the ResponseBuffer struct definition for a description of
 // how this works)
 func (rb *ResponseBuffer) StartNestedBatch(batchType string, params ...string) (batchID string) {
+	if !rb.session.capabilities.Has(caps.Batch) {
+		return
+	}
 	batchID = rb.session.generateBatchID()
 	msgParams := make([]string, len(params)+2)
 	msgParams[0] = "+" + batchID
@@ -219,19 +222,6 @@ func (rb *ResponseBuffer) EndNestedBatch(batchID string) {
 	rb.AddMessage(ircmsg.MakeMessage(nil, rb.target.server.name, "BATCH", "-"+batchID))
 }
 
-// Convenience to start a nested batch for history lines, at the highest level
-// supported by the client (`history`, `chathistory`, or no batch, in descending order).
-func (rb *ResponseBuffer) StartNestedHistoryBatch(params ...string) (batchID string) {
-	var batchType string
-	if rb.session.capabilities.Has(caps.Batch) {
-		batchType = "chathistory"
-	}
-	if batchType != "" {
-		batchID = rb.StartNestedBatch(batchType, params...)
-	}
-	return
-}
-
 // Send sends all messages in the buffer to the client.
 // Afterwards, the buffer is in an undefined state and MUST NOT be used further.
 // If `blocking` is true you MUST be sending to the client from its own goroutine.

irc/utils/types.go

@@ -20,6 +20,14 @@ func (s HashSet[T]) Remove(elem T) {
 	delete(s, elem)
 }
 
+func SetLiteral[T comparable](elems ...T) HashSet[T] {
+	result := make(HashSet[T], len(elems))
+	for _, elem := range elems {
+		result.Add(elem)
+	}
+	return result
+}
+
 func CopyMap[K comparable, V any](input map[K]V) (result map[K]V) {
 	result = make(map[K]V, len(input))
 	for key, value := range input {

irctest

@@ -1 +1 @@
-Subproject commit 22c6743b24f2a85bf79a92fb0c7fab325c047a6c
+Subproject commit 5a5dbdb50dda3dc9e5ed3d542ab75b1b87fccac9