Repository: oragono/oragono (go)

Issue:
help numerics should include the nick
@kylef reports: "I noticed that help output (704/705/706) numerics do not contain client nick unlike other numerics in Oragono, and these numerics on other servers (ratbox/inspircd etc). Is that a bug?"

Answer: almost certainly yes

Pull request description:
fix #1472
```
helpop privmsg
:oragono.test 704 nickname PRIVMSG :PRIVMSG <target>{,<target>} <text to be sent>
:oragono.test 705 nickname PRIVMSG :
:oragono.test 705 nickname PRIVMSG :Sends the text to the given targets as a PRIVMSG.
:oragono.test 706 nickname PRIVMSG :End of /HELPOP
```

Patch:
diff --git a/irc/handlers.go b/irc/handlers.go
index a1756a25..f461d26f 100644
--- a/irc/handlers.go
+++ b/irc/handlers.go
@@ -1015,16 +1015,17 @@ func extjwtHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re
 }
 
 // HELP [<query>]
+// HELPOP [<query>]
 func helpHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *ResponseBuffer) bool {
-	argument := strings.ToLower(strings.TrimSpace(strings.Join(msg.Params, " ")))
-
-	if len(argument) < 1 {
+	if len(msg.Params) == 0 {
 		client.sendHelp("HELPOP", client.t(`HELPOP <argument>
 
 Get an explanation of <argument>, or "index" for a list of help topics.`), rb)
 		return false
 	}
 
+	argument := strings.ToLower(strings.TrimSpace(msg.Params[0]))
+
 	// handle index
 	if argument == "index" {
 		client.sendHelp("HELP", server.helpIndexManager.GetIndex(client.Languages(), client.HasMode(modes.Operator)), rb)
@@ -1035,14 +1036,12 @@ Get an explanation of <argument>, or "index" for a list of help topics.`), rb)
 
 	if exists && (!helpHandler.oper || (helpHandler.oper && client.HasMode(modes.Operator))) {
 		if helpHandler.textGenerator != nil {
-			client.sendHelp(strings.ToUpper(argument), helpHandler.textGenerator(client), rb)
+			client.sendHelp(argument, helpHandler.textGenerator(client), rb)
 		} else {
-			client.sendHelp(strings.ToUpper(argument), client.t(helpHandler.text), rb)
+			client.sendHelp(argument, client.t(helpHandler.text), rb)
 		}
 	} else {
-		args := msg.Params
-		args = append(args, client.t("Help not found"))
-		rb.Add(nil, server.name, ERR_HELPNOTFOUND, args...)
+		rb.Add(nil, server.name, ERR_HELPNOTFOUND, client.Nick(), strings.ToUpper(utils.SafeErrorParam(argument)), client.t("Help not found"))
 	}
 
 	return false
@@ -1865,7 +1864,7 @@ func monitorListHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 		nickList = append(nickList, replynick)
 	}
 
-	for _, line := range utils.ArgsToStrings(maxLastArgLength, nickList, ",") {
+	for _, line := range utils.BuildTokenLines(maxLastArgLength, nickList, ",") {
 		rb.Add(nil, server.name, RPL_MONLIST, nick, line)
 	}
 
@@ -1891,12 +1890,12 @@ func monitorStatusHandler(server *Server, client *Client, msg ircmsg.IrcMessage,
 	}
 
 	if len(online) > 0 {
-		for _, line := range utils.ArgsToStrings(maxLastArgLength, online, ",") {
+		for _, line := range utils.BuildTokenLines(maxLastArgLength, online, ",") {
 			rb.Add(nil, server.name, RPL_MONONLINE, client.Nick(), line)
 		}
 	}
 	if len(offline) > 0 {
-		for _, line := range utils.ArgsToStrings(maxLastArgLength, offline, ",") {
+		for _, line := range utils.BuildTokenLines(maxLastArgLength, offline, ",") {
 			rb.Add(nil, server.name, RPL_MONOFFLINE, client.Nick(), line)
 		}
 	}
diff --git a/irc/help.go b/irc/help.go
index 5ff28702..0e12d7d6 100644
--- a/irc/help.go
+++ b/irc/help.go
@@ -752,22 +752,19 @@ func (hm *HelpIndexManager) GenerateIndices(lm *languages.Manager) {
 }
 
 // sendHelp sends the client help of the given string.
-func (client *Client) sendHelp(name string, text string, rb *ResponseBuffer) {
-	splitName := strings.Split(name, " ")
+func (client *Client) sendHelp(helpEntry string, text string, rb *ResponseBuffer) {
+	helpEntry = strings.ToUpper(helpEntry)
+	nick := client.Nick()
 	textLines := strings.Split(text, "\n")
 
 	for i, line := range textLines {
-		args := splitName
-		args = append(args, line)
 		if i == 0 {
-			rb.Add(nil, client.server.name, RPL_HELPSTART, args...)
+			rb.Add(nil, client.server.name, RPL_HELPSTART, nick, helpEntry, line)
 		} else {
-			rb.Add(nil, client.server.name, RPL_HELPTXT, args...)
+			rb.Add(nil, client.server.name, RPL_HELPTXT, nick, helpEntry, line)
 		}
 	}
-	args := splitName
-	args = append(args, client.t("End of /HELPOP"))
-	rb.Add(nil, client.server.name, RPL_ENDOFHELP, args...)
+	rb.Add(nil, client.server.name, RPL_ENDOFHELP, nick, helpEntry, client.t("End of /HELPOP"))
 }
 
 // GetHelpIndex returns the help index for the given language.
diff --git a/irc/utils/args.go b/irc/utils/args.go
index 20884ef3..a80bbab5 100644
--- a/irc/utils/args.go
+++ b/irc/utils/args.go
@@ -18,37 +18,6 @@ var (
 	ErrInvalidParams = errors.New("Invalid parameters")
 )
 
-// ArgsToStrings takes the arguments and splits them into a series of strings,
-// each argument separated by delim and each string bounded by maxLength.
-func ArgsToStrings(maxLength int, arguments []string, delim string) []string {
-	var messages []string
-
-	var buffer string
-	for {
-		if len(arguments) < 1 {
-			break
-		}
-
-		if len(buffer) > 0 && maxLength < len(buffer)+len(delim)+len(arguments[0]) {
-			messages = append(messages, buffer)
-			buffer = ""
-			continue
-		}
-
-		if len(buffer) > 0 {
-			buffer += delim
-		}
-		buffer += arguments[0]
-		arguments = arguments[1:]
-	}
-
-	if len(buffer) > 0 {
-		messages = append(messages, buffer)
-	}
-
-	return messages
-}
-
 func StringToBool(str string) (result bool, err error) {
 	switch strings.ToLower(str) {
 	case "on", "true", "t", "yes", "y", "enabled":
diff --git a/irc/utils/text.go b/irc/utils/text.go
index bffa7a34..d9104564 100644
--- a/irc/utils/text.go
+++ b/irc/utils/text.go
@@ -132,3 +132,14 @@ func (t *TokenLineBuilder) Lines() (result []string) {
 	}
 	return
 }
+
+// BuildTokenLines is a convenience to apply TokenLineBuilder to a predetermined
+// slice of tokens.
+func BuildTokenLines(lineLen int, tokens []string, delim string) []string {
+	var tl TokenLineBuilder
+	tl.Initialize(lineLen, delim)
+	for _, arg := range tokens {
+		tl.Add(arg)
+	}
+	return tl.Lines()
+}


Test patch:
diff --git a/irc/utils/args_test.go b/irc/utils/args_test.go
index b846d122..5c84b26a 100644
--- a/irc/utils/args_test.go
+++ b/irc/utils/args_test.go
@@ -5,14 +5,6 @@ package utils
 
 import "testing"
 
-func TestArgsToStrings(t *testing.T) {
-	val := ArgsToStrings(512, []string{"a", "b", "c"}, ",")
-	assertEqual(val, []string{"a,b,c"}, t)
-
-	val = ArgsToStrings(10, []string{"abcd", "efgh", "ijkl"}, ",")
-	assertEqual(val, []string{"abcd,efgh", "ijkl"}, t)
-}
-
 func TestStringToBool(t *testing.T) {
 	val, err := StringToBool("on")
 	assertEqual(val, true, t)
diff --git a/irc/utils/text_test.go b/irc/utils/text_test.go
index 68e23946..6aa05e76 100644
--- a/irc/utils/text_test.go
+++ b/irc/utils/text_test.go
@@ -35,3 +35,11 @@ func TestTokenLineBuilder(t *testing.T) {
 		t.Errorf("text incorrectly split into lines: %s instead of %s", joined, monteCristo)
 	}
 }
+
+func TestBuildTokenLines(t *testing.T) {
+	val := BuildTokenLines(512, []string{"a", "b", "c"}, ",")
+	assertEqual(val, []string{"a,b,c"}, t)
+
+	val = BuildTokenLines(10, []string{"abcd", "efgh", "ijkl"}, ",")
+	assertEqual(val, []string{"abcd,efgh", "ijkl"}, t)
+}