updates to be better

Author: lukegalbraithrussell

cmd/docs/docs.go

@@ -19,6 +19,7 @@ import (
 	"net/url"
 
 	"github.com/slackapi/slack-cli/internal/shared"
+	"github.com/slackapi/slack-cli/internal/slackerror"
 	"github.com/slackapi/slack-cli/internal/slacktrace"
 	"github.com/slackapi/slack-cli/internal/style"
 	"github.com/spf13/cobra"
@@ -37,25 +38,20 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command {
 				Command: "docs",
 			},
 			{
-				Meaning: "Open Slack docs search page",
-				Command: "docs --search",
-			},
-			{
-				Meaning: "Search Slack docs",
+				Meaning: "Search Slack developer docs for Block Kit",
 				Command: "docs --search \"Block Kit\"",
 			},
 			{
-				Meaning: "Search Slack docs without search flag",
-				Command: "docs \"Block Kit\"",
+				Meaning: "Open Slack docs search page",
+				Command: "docs --search",
 			},
 		}),
-		Args: cobra.MaximumNArgs(1), // Allow 0-1 arguments for search query
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return runDocsCommand(clients, cmd, args)
 		},
 	}
 
-	cmd.Flags().BoolVar(&searchMode, "search", false, "open Slack docs search page")
+	cmd.Flags().BoolVar(&searchMode, "search", false, "open Slack docs search page or search with query")
 
 	return cmd
 }
@@ -67,17 +63,27 @@ func runDocsCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []st
 	var docsURL string
 	var sectionText string
 
-	if len(args) > 0 {
-		// Search query provided as positional argument: slack docs "query"
-		searchQuery := url.QueryEscape(args[0])
-		docsURL = fmt.Sprintf("https://docs.slack.dev/search/?q=%s", searchQuery)
-		sectionText = "Docs Search"
-	} else if searchMode {
-		// Search flag provided without query: slack docs --search
-		docsURL = "https://docs.slack.dev/search/"
-		sectionText = "Docs Search"
+	// Validate: if there are arguments, --search flag must be used
+	if len(args) > 0 && !cmd.Flags().Changed("search") {
+		return slackerror.New(slackerror.ErrDocsSearchFlagRequired).WithRemediation(
+			"Use --search flag: %s", 
+			style.Commandf(fmt.Sprintf("docs --search \"%s\"", args[0]), false),
+		)
+	}
+
+	if cmd.Flags().Changed("search") {
+		if len(args) > 0 {
+			// --search "query" (space-separated) - use the first arg as the query
+			encodedQuery := url.QueryEscape(args[0])
+			docsURL = fmt.Sprintf("https://docs.slack.dev/search/?q=%s", encodedQuery)
+			sectionText = "Docs Search"
+		} else {
+			// --search (no argument) - open search page
+			docsURL = "https://docs.slack.dev/search/"
+			sectionText = "Docs Search"
+		}
 	} else {
-		// Default homepage: slack docs
+		// No search flag: default homepage
 		docsURL = "https://docs.slack.dev"
 		sectionText = "Docs Open"
 	}
@@ -92,10 +98,10 @@ func runDocsCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []st
 
 	clients.Browser().OpenURL(docsURL)
 
-	if len(args) > 0 || searchMode {
+	if cmd.Flags().Changed("search") {
 		traceValue := ""
 		if len(args) > 0 {
-			traceValue = args[0]
+			traceValue = args[0] // For space-separated syntax
 		}
 		clients.IO.PrintTrace(ctx, slacktrace.DocsSearchSuccess, traceValue)
 	} else {

cmd/docs/docs_test.go

@@ -40,8 +40,16 @@ func Test_Docs_DocsCommand(t *testing.T) {
 				"https://docs.slack.dev",
 			},
 		},
-		"opens docs with basic search query": {
-			CmdArgs: []string{"messaging"},
+		"fails when positional argument provided without search flag": {
+			CmdArgs:              []string{"Block Kit"},
+			ExpectedErrorStrings: []string{"Invalid docs command. Did you mean to search?"},
+			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
+				// No browser calls should be made when command fails
+				cm.Browser.AssertNotCalled(t, "OpenURL")
+			},
+		},
+		"opens docs with search query using space syntax": {
+			CmdArgs: []string{"--search", "messaging"},
 			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
 				expectedURL := "https://docs.slack.dev/search/?q=messaging"
 				cm.Browser.AssertCalled(t, "OpenURL", expectedURL)
@@ -53,7 +61,7 @@ func Test_Docs_DocsCommand(t *testing.T) {
 			},
 		},
 		"handles search query with multiple words": {
-			CmdArgs: []string{"socket mode"},
+			CmdArgs: []string{"--search", "socket mode"},
 			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
 				expectedURL := "https://docs.slack.dev/search/?q=socket+mode"
 				cm.Browser.AssertCalled(t, "OpenURL", expectedURL)
@@ -65,7 +73,7 @@ func Test_Docs_DocsCommand(t *testing.T) {
 			},
 		},
 		"handles special characters in search query": {
-			CmdArgs: []string{"messages & webhooks"},
+			CmdArgs: []string{"--search", "messages & webhooks"},
 			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
 				expectedURL := "https://docs.slack.dev/search/?q=messages+%26+webhooks"
 				cm.Browser.AssertCalled(t, "OpenURL", expectedURL)
@@ -77,7 +85,7 @@ func Test_Docs_DocsCommand(t *testing.T) {
 			},
 		},
 		"handles search query with quotes": {
-			CmdArgs: []string{"webhook \"send message\""},
+			CmdArgs: []string{"--search", "webhook \"send message\""},
 			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
 				expectedURL := "https://docs.slack.dev/search/?q=webhook+%22send+message%22"
 				cm.Browser.AssertCalled(t, "OpenURL", expectedURL)
@@ -88,18 +96,6 @@ func Test_Docs_DocsCommand(t *testing.T) {
 				"https://docs.slack.dev/search/?q=webhook+%22send+message%22",
 			},
 		},
-		"handles empty search query": {
-			CmdArgs: []string{""},
-			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
-				expectedURL := "https://docs.slack.dev/search/?q="
-				cm.Browser.AssertCalled(t, "OpenURL", expectedURL)
-				cm.IO.AssertCalled(t, "PrintTrace", mock.Anything, slacktrace.DocsSearchSuccess, mock.Anything)
-			},
-			ExpectedOutputs: []string{
-				"Docs Search",
-				"https://docs.slack.dev/search/?q=",
-			},
-		},
 		"handles search flag without argument": {
 			CmdArgs: []string{"--search"},
 			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {

internal/slackerror/errors.go

@@ -96,6 +96,7 @@ const (
 	ErrDenoNotFound                                  = "deno_not_found"
 	ErrDeployedAppNotSupported                       = "deployed_app_not_supported"
 	ErrDocumentationGenerationFailed                 = "documentation_generation_failed"
+	ErrDocsSearchFlagRequired                        = "docs_search_flag_required"
 	ErrEnterpriseNotFound                            = "enterprise_not_found"
 	ErrFailedAddingCollaborator                      = "failed_adding_collaborator"
 	ErrFailedCreatingApp                             = "failed_creating_app"
@@ -679,6 +680,12 @@ Otherwise start your app for local development with: %s`,
 		Message: "Failed to generate documentation",
 	},
 
+	ErrDocsSearchFlagRequired: {
+		Code:        ErrDocsSearchFlagRequired,
+		Message:     "Invalid docs command. Did you mean to search?",
+		Remediation: fmt.Sprintf("Use --search flag: %s", style.Commandf("docs --search \"<query>\"", false)),
+	},
+
 	ErrEnterpriseNotFound: {
 		Code:    ErrEnterpriseNotFound,
 		Message: "The `enterprise` was not found",