Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 15, 2025

Replaces string slice allocation and strings.Join() with strings.Builder pattern to match the style used in generate-docs.go.

Before:

defaultStrings := make([]string, len(defaultIDs))
for i, id := range defaultIDs {
    defaultStrings[i] = string(id)
}
defaultTools := strings.Join(defaultStrings, ", ")

toolsetsHelp := fmt.Sprintf("...%s\n", availableTools) +
    "..." +
    fmt.Sprintf("...%s\n", defaultTools) +
    "..."

After:

var defaultBuf strings.Builder
for i, id := range defaultIDs {
    if i > 0 {
        defaultBuf.WriteString(", ")
    }
    defaultBuf.WriteString(string(id))
}

var buf strings.Builder
buf.WriteString("...")
buf.WriteString(defaultBuf.String())
buf.WriteString("...")
  • Eliminates intermediate slice allocations
  • Removes unused fmt import
  • Adds TestGenerateToolsetsHelp() to verify output contains expected sections and toolsets
  • Output format unchanged
Original prompt

GenerateToolsetsHelp() could use the nice string builder like we see in generate-docs, can you check for any significant string building, and write it in that way, rather than doing the current ugly string list building, concatenation and slice joining?


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor string building in GenerateToolsetsHelp method Refactor GenerateToolsetsHelp() to use strings.Builder Dec 15, 2025
Copilot AI requested a review from SamMorrowDrums December 15, 2025 22:45
@SamMorrowDrums SamMorrowDrums marked this pull request as ready for review December 16, 2025 18:49
@SamMorrowDrums SamMorrowDrums requested a review from a team as a code owner December 16, 2025 18:49
Copilot AI review requested due to automatic review settings December 16, 2025 18:49
@SamMorrowDrums SamMorrowDrums merged commit 178d9c4 into SamMorrowDrums/registry-builder-pattern Dec 16, 2025
25 of 27 checks passed
@SamMorrowDrums SamMorrowDrums deleted the copilot/refactor-string-building-in-toolsets branch December 16, 2025 18:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors GenerateToolsetsHelp() to use the strings.Builder pattern instead of slice allocation and strings.Join(), aligning with the code style used in generate-docs.go.

Key Changes:

  • Replaces slice-based string building with strings.Builder for better performance and cleaner code
  • Removes unused fmt import
  • Adds comprehensive test coverage for GenerateToolsetsHelp() to verify output format

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/github/tools.go Refactored string building logic from slice+join to strings.Builder pattern; removed fmt import; output format unchanged
pkg/github/tools_test.go Added TestGenerateToolsetsHelp() with assertions for expected sections, default toolsets, and available toolsets

case i == 0:
currentLine = id
case len(currentLine)+len(id)+2 <= maxLineLength:
currentLine += ", " + id
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 290 still uses string concatenation (+=) to build the currentLine. For consistency with the strings.Builder refactoring pattern, consider using a strings.Builder for building currentLine as well, or document why the current approach is preferred for this specific case.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants