From 055546087b1284c7152b8495ddf1059c40d91f68 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Wed, 13 Aug 2025 23:21:10 -0700 Subject: [PATCH 1/2] fix: generate documentation without template literals and with standard headers --- cmd/docgen/docgen.go | 158 ++++++++++++++++++++++++++++++- cmd/docgen/docgen_test.go | 31 ------ internal/shared/clients.go | 15 --- internal/shared/clients_mock.go | 4 - internal/slackdeps/cobra_mock.go | 41 -------- internal/slackerror/errors.go | 14 ++- 6 files changed, 166 insertions(+), 97 deletions(-) delete mode 100644 internal/slackdeps/cobra_mock.go diff --git a/cmd/docgen/docgen.go b/cmd/docgen/docgen.go index b04f886b..c46df679 100644 --- a/cmd/docgen/docgen.go +++ b/cmd/docgen/docgen.go @@ -15,15 +15,19 @@ package docgen import ( + "bytes" _ "embed" "fmt" + "io" "path/filepath" + "slices" "strings" "text/template" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" + "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -99,9 +103,9 @@ func runDocGenCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg return slackerror.New("MkdirAll failed").WithRootCause(err) } rootCmd.DisableAutoGenTag = true - err = clients.Cobra.GenMarkdownTree(rootCmd, commandsDirPath) + err = genMarkdownTree(rootCmd, clients.Fs, commandsDirPath) if err != nil { - return slackerror.New("Cobra.GenMarkdownTree failed").WithRootCause(err) + return slackerror.New(slackerror.ErrDocumentationGenerationFailed).WithRootCause(err) } // Generate errors reference @@ -124,3 +128,153 @@ func runDocGenCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg )) return nil } + +// genMarkdownTree creates markdown reference of commands for the docs site. +// +// Reference: https://github.com/spf13/cobra/blob/3f3b81882534a51628f3286e93c6842d9b2e29ea/doc/md_docs.go#L119-L158 +func genMarkdownTree(cmd *cobra.Command, fs afero.Fs, dir string) error { + for _, c := range cmd.Commands() { + if !c.IsAvailableCommand() || c.IsAdditionalHelpTopicCommand() { + continue + } + if err := genMarkdownTree(c, fs, dir); err != nil { + return err + } + } + basename := strings.ReplaceAll(cmd.CommandPath(), " ", "_") + ".md" + filename := filepath.Join(dir, basename) + f, err := fs.Create(filename) + if err != nil { + return err + } + defer f.Close() + if err := genMarkdownCommand(cmd, f); err != nil { + return err + } + return nil +} + +// genMarkdownCommand creates custom markdown output for a command. +// +// Reference: https://github.com/spf13/cobra/blob/3f3b81882534a51628f3286e93c6842d9b2e29ea/doc/md_docs.go#L56-L117 +func genMarkdownCommand(cmd *cobra.Command, w io.Writer) error { + cmd.InitDefaultHelpCmd() + cmd.InitDefaultHelpFlag() + + buf := new(bytes.Buffer) + name := cmd.CommandPath() + + fmt.Fprintf(buf, "# `%s`\n\n", name) + fmt.Fprintf(buf, "%s\n\n", cmd.Short) + if len(cmd.Long) > 0 { + fmt.Fprintf(buf, "## Description\n\n") + description, err := render(cmd.Long) + if err != nil { + return err + } + fmt.Fprintf(buf, "%s\n\n", description) + } + if cmd.Runnable() { + fmt.Fprintf(buf, "```\n%s\n```\n\n", cmd.UseLine()) + } + if len(cmd.Example) > 0 { + fmt.Fprintf(buf, "## Examples\n\n") + fmt.Fprintf(buf, "```\n%s\n```\n\n", cmd.Example) + } + if err := genMarkdownCommandFlags(buf, cmd); err != nil { + return err + } + if hasSeeAlso(cmd) { + fmt.Fprintf(buf, "## See also\n\n") + if cmd.HasParent() { + parent := cmd.Parent() + pname := parent.CommandPath() + link := strings.ReplaceAll(pname, " ", "_") + fmt.Fprintf(buf, "* [%s](%s)\t - %s\n", pname, link, parent.Short) + cmd.VisitParents(func(c *cobra.Command) { + if c.DisableAutoGenTag { + cmd.DisableAutoGenTag = c.DisableAutoGenTag + } + }) + } + children := cmd.Commands() + slices.SortFunc(children, func(a *cobra.Command, b *cobra.Command) int { + if a.Name() < b.Name() { + return -1 + } else { + return 1 + } + }) + for _, child := range children { + if !child.IsAvailableCommand() || child.IsAdditionalHelpTopicCommand() { + continue + } + cname := name + " " + child.Name() + link := strings.ReplaceAll(cname, " ", "_") + fmt.Fprintf(buf, "* [%s](%s)\t - %s\n", cname, link, child.Short) + } + fmt.Fprintf(buf, "\n") + } + _, err := buf.WriteTo(w) + return err +} + +// genMarkdownCommandFlags outputs flag information. +// +// Reference: https://github.com/spf13/cobra/blob/3f3b81882534a51628f3286e93c6842d9b2e29ea/doc/md_docs.go#L32-L49 +func genMarkdownCommandFlags(buf *bytes.Buffer, cmd *cobra.Command) error { + flags := cmd.NonInheritedFlags() + flags.SetOutput(buf) + if flags.HasAvailableFlags() { + fmt.Fprintf(buf, "## Flags\n\n```\n") + flags.PrintDefaults() + fmt.Fprintf(buf, "```\n\n") + } + parentFlags := cmd.InheritedFlags() + parentFlags.SetOutput(buf) + if parentFlags.HasAvailableFlags() { + fmt.Fprintf(buf, "## Global flags\n\n```\n") + parentFlags.PrintDefaults() + fmt.Fprintf(buf, "```\n\n") + } + return nil +} + +// hasSeeAlso checks for adjancet commands to include in reference. +// +// Reference: https://github.com/spf13/cobra/blob/3f3b81882534a51628f3286e93c6842d9b2e29ea/doc/util.go#L23-L37 +func hasSeeAlso(cmd *cobra.Command) bool { + if cmd.HasParent() { + return true + } + for _, c := range cmd.Commands() { + if !c.IsAvailableCommand() || c.IsAdditionalHelpTopicCommand() { + continue + } + return true + } + return false +} + +// render formats the templating from a command into markdown. +func render(input string) (string, error) { + tmpl, err := template.New("md").Funcs(template.FuncMap{ + "Emoji": func(s string) string { + return "" + }, + "LinkText": func(s string) string { + return fmt.Sprintf("[%s](%s)", s, s) + }, + "ToBold": func(s string) string { + return fmt.Sprintf("**%s**", s) + }, + }).Parse(input) + if err != nil { + return "", err + } + var buf bytes.Buffer + if err := tmpl.Execute(&buf, nil); err != nil { + return "", err + } + return buf.String(), nil +} diff --git a/cmd/docgen/docgen_test.go b/cmd/docgen/docgen_test.go index 67752b3c..452c01ee 100644 --- a/cmd/docgen/docgen_test.go +++ b/cmd/docgen/docgen_test.go @@ -40,12 +40,6 @@ func TestNewDocsCommand(t *testing.T) { filepath.Join(slackdeps.MockWorkingDirectory, "docs"), }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.Cobra.AssertCalled( - t, - "GenMarkdownTree", - mock.Anything, - filepath.Join(slackdeps.MockWorkingDirectory, "docs", "commands"), - ) cm.Fs.AssertCalled( t, "MkdirAll", @@ -84,12 +78,6 @@ func TestNewDocsCommand(t *testing.T) { filepath.Join(slackdeps.MockWorkingDirectory, "markdown-docs"), }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.Cobra.AssertCalled( - t, - "GenMarkdownTree", - mock.Anything, - filepath.Join(slackdeps.MockWorkingDirectory, "markdown-docs", "commands"), - ) cm.Fs.AssertCalled( t, "MkdirAll", @@ -126,12 +114,6 @@ func TestNewDocsCommand(t *testing.T) { CmdArgs: []string{}, ExpectedOutputs: []string{"References saved to: docs"}, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.Cobra.AssertCalled( - t, - "GenMarkdownTree", - mock.Anything, - filepath.Join("docs", "commands"), - ) cm.Fs.AssertCalled( t, "MkdirAll", @@ -185,19 +167,6 @@ func TestNewDocsCommand(t *testing.T) { CmdArgs: []string{}, ExpectedErrorStrings: []string{"no write permission"}, }, - "when generating docs fails": { - Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - cm.Cobra.On( - "GenMarkdownTree", - mock.Anything, - mock.Anything, - ).Return( - errors.New("failed to generate docs"), - ) - }, - CmdArgs: []string{}, - ExpectedErrorStrings: []string{"failed to generate docs"}, - }, }, func(clients *shared.ClientFactory) *cobra.Command { return NewCommand(clients) }) diff --git a/internal/shared/clients.go b/internal/shared/clients.go index 09818b4b..7bf0e7b2 100644 --- a/internal/shared/clients.go +++ b/internal/shared/clients.go @@ -38,8 +38,6 @@ import ( "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/tracking" "github.com/spf13/afero" - "github.com/spf13/cobra" - "github.com/spf13/cobra/doc" ) // ClientFactory are shared clients and configurations for use across the CLI commands (cmd) and handlers (pkg). @@ -66,14 +64,6 @@ type ClientFactory struct { // CleanupWaitGroup is a group of wait groups shared by all packages and allow functions to cleanup before the process terminates CleanupWaitGroup sync.WaitGroup - - // Cobra are a group of Cobra functions shared by all packages and enables tests & mocking - Cobra struct { - // GenMarkdownTree defaults to `doc.GenMarkdownTree(...)` and can be mocked to test specific use-cases - // TODO - This can be moved to cmd/docs/docs.go when `NewCommand` returns an instance of that can store `GenMarkdownTree` as - // a private member. The current thinking is that `NewCommand` would return a `SlackCommand` instead of `CobraCommand` - GenMarkdownTree func(cmd *cobra.Command, dir string) error - } } const sdkSlackDevDomainFlag = "sdk-slack-dev-domain" @@ -102,11 +92,6 @@ func NewClientFactory(options ...func(*ClientFactory)) *ClientFactory { clients.Auth = clients.defaultAuthFunc clients.Browser = clients.defaultBrowserFunc - // Command-specific dependencies - // TODO - These are methods that belong to specific commands and should be moved under each command - // when we replace NewCommand with NewSlackCommand that can store member variables. - clients.Cobra.GenMarkdownTree = doc.GenMarkdownTree - // TODO: Temporary hack to get around circular dependency in internal/api/client.go since that imports version // Follows pattern demonstrated by the GitHub CLI here https://github.com/cli/cli/blob/5a46c1cab601a3394caa8de85adb14f909b811e9/pkg/cmd/factory/default.go#L29 // Used by the APIClient for its userAgent diff --git a/internal/shared/clients_mock.go b/internal/shared/clients_mock.go index ef51048e..bc3b3156 100644 --- a/internal/shared/clients_mock.go +++ b/internal/shared/clients_mock.go @@ -37,7 +37,6 @@ type ClientsMock struct { AppClient *app.Client Browser *slackdeps.BrowserMock Config *config.Config - Cobra *slackdeps.CobraMock EventTracker *tracking.EventTrackerMock Fs *slackdeps.FsMock IO *iostreams.IOStreamsMock @@ -55,7 +54,6 @@ func NewClientsMock() *ClientsMock { clientsMock.API = &api.APIMock{} clientsMock.Auth = &auth.AuthMock{} clientsMock.Browser = slackdeps.NewBrowserMock() - clientsMock.Cobra = slackdeps.NewCobraMock() clientsMock.EventTracker = &tracking.EventTrackerMock{} clientsMock.Fs = slackdeps.NewFsMock() clientsMock.Os = slackdeps.NewOsMock() @@ -74,7 +72,6 @@ func (m *ClientsMock) AddDefaultMocks() { m.API.AddDefaultMocks() m.Auth.AddDefaultMocks() m.Browser.AddDefaultMocks() - m.Cobra.AddDefaultMocks() m.EventTracker.AddDefaultMocks() m.IO.AddDefaultMocks() m.Os.AddDefaultMocks() @@ -85,7 +82,6 @@ func (m *ClientsMock) AddDefaultMocks() { func (m *ClientsMock) MockClientFactory() func(c *ClientFactory) { return func(clients *ClientFactory) { clients.Browser = func() slackdeps.Browser { return m.Browser } - clients.Cobra.GenMarkdownTree = m.Cobra.GenMarkdownTree clients.Config = m.Config clients.EventTracker = m.EventTracker clients.Os = m.Os diff --git a/internal/slackdeps/cobra_mock.go b/internal/slackdeps/cobra_mock.go deleted file mode 100644 index aea30929..00000000 --- a/internal/slackdeps/cobra_mock.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2022-2025 Salesforce, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package slackdeps - -import ( - "github.com/spf13/cobra" - "github.com/stretchr/testify/mock" -) - -// NewCobraMock creates a mock for some Cobra functions. -func NewCobraMock() *CobraMock { - return &CobraMock{} -} - -// CobraMock mocks the client's Cobra struct. -type CobraMock struct { - mock.Mock -} - -// AddDefaultMocks installs the default mock actions to fallback on. -func (m *CobraMock) AddDefaultMocks() { - m.On("GenMarkdownTree", mock.Anything, mock.Anything).Return(nil) -} - -// GenMarkdownTree mock. -func (m *CobraMock) GenMarkdownTree(cmd *cobra.Command, dir string) error { - args := m.Called(cmd, dir) - return args.Error(0) -} diff --git a/internal/slackerror/errors.go b/internal/slackerror/errors.go index a552e84d..0b773eb6 100644 --- a/internal/slackerror/errors.go +++ b/internal/slackerror/errors.go @@ -95,6 +95,7 @@ const ( ErrDefaultAppSetting = "default_app_setting_error" ErrDenoNotFound = "deno_not_found" ErrDeployedAppNotSupported = "deployed_app_not_supported" + ErrDocumentationGenerationFailed = "documentation_generation_failed" ErrEnterpriseNotFound = "enterprise_not_found" ErrFailedAddingCollaborator = "failed_adding_collaborator" ErrFailedCreatingApp = "failed_creating_app" @@ -320,7 +321,7 @@ var ErrorCodeMap = map[string]Error{ ErrAppAuthTeamMismatch: { Code: ErrAppAuthTeamMismatch, Message: "Specified app and team are mismatched", - Remediation: "Try a different combination of --app, --team flags", + Remediation: "Try a different combination of `--app` and `--team` flags", }, ErrAppCreate: { @@ -374,7 +375,7 @@ Otherwise start your app for local development with: %s`, ErrAppFlagRequired: { Code: ErrAppFlagRequired, Message: "The --app flag must be provided", - Remediation: fmt.Sprintf("Choose a specific app with %s", style.Highlight("--app ")), + Remediation: "Choose a specific app with `--app `", }, ErrAppFound: { @@ -673,6 +674,11 @@ Otherwise start your app for local development with: %s`, Message: "A deployed app cannot be used by this command", }, + ErrDocumentationGenerationFailed: { + Code: ErrDocumentationGenerationFailed, + Message: "Failed to generate documentation", + }, + ErrEnterpriseNotFound: { Code: ErrEnterpriseNotFound, Message: "The `enterprise` was not found", @@ -849,7 +855,7 @@ Otherwise start your app for local development with: %s`, ErrInvalidAppFlag: { Code: ErrInvalidAppFlag, Message: "The provided --app flag value is not valid", - Remediation: fmt.Sprintf("Specify the environment with %s or %s\nOr choose a specific app with %s", style.Highlight("--app local"), style.Highlight("--app deployed"), style.Highlight("--app ")), + Remediation: "Specify the environment with `--app local` or `--app deployed`\nOr choose a specific app with `--app `", }, ErrInvalidAppID: { @@ -1408,7 +1414,7 @@ Otherwise start your app for local development with: %s`, ErrTeamFlagRequired: { Code: ErrTeamFlagRequired, Message: "The --team flag must be provided", - Remediation: fmt.Sprintf("Choose a specific team with %s or %s", style.Highlight("--team "), style.Highlight("--team ")), + Remediation: "Choose a specific team with `--team ` or `--team `", }, ErrTeamList: { From 7aff432df0c25de5a4be23ac7e8e761a9962063b Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Wed, 13 Aug 2025 23:43:03 -0700 Subject: [PATCH 2/2] docs: list flags before examples in generated reference --- cmd/docgen/docgen.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/docgen/docgen.go b/cmd/docgen/docgen.go index c46df679..12203b05 100644 --- a/cmd/docgen/docgen.go +++ b/cmd/docgen/docgen.go @@ -177,13 +177,13 @@ func genMarkdownCommand(cmd *cobra.Command, w io.Writer) error { if cmd.Runnable() { fmt.Fprintf(buf, "```\n%s\n```\n\n", cmd.UseLine()) } + if err := genMarkdownCommandFlags(buf, cmd); err != nil { + return err + } if len(cmd.Example) > 0 { fmt.Fprintf(buf, "## Examples\n\n") fmt.Fprintf(buf, "```\n%s\n```\n\n", cmd.Example) } - if err := genMarkdownCommandFlags(buf, cmd); err != nil { - return err - } if hasSeeAlso(cmd) { fmt.Fprintf(buf, "## See also\n\n") if cmd.HasParent() {